docs: add call condition note for TSUrlHostGet - #13313
Conversation
Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK. For earlier hooks like TS_HTTP_READ_REQUEST_HDR_HOOK, TSHttpHdrHostGet should be used instead. Fixes apache#5742
|
Hello! This is my first contribution to Apache Traffic Server. Could you please trigger CI for this documentation PR? Thank you! |
|
@bneradt The Docs job failed due to warnings (warnings are treated as errors), but the warnings I found were related to env files. I'm not sure what the issue was. Could you take a look? |
|
From the docs build: |
|
Hi, just checking in on this docs PR. It adds a call condition note for TSUrlHostGet. Would appreciate a review when you have time. Thanks! |
Thank you for contributing, however you likely won't get a review until the docs build passes. Please see my previous comment showing the failure output. |
Replace :data: with double backticks for hook names, consistent with the rest of the documentation. Fixes apache#5742
|
Hi @bneradt, I've fixed the |
|
@mmustafasenoglu Once you are authorized to start Jenkins jobs (@bneradt added you to the whitelist) they will start automatically when you open a PR or push to it. |
|
Hi! Just following up on this documentation PR. Added a call condition note for TSUrlHostGet specifying it should only be called after TS_HTTP_POST_REMAP_HOOK. Let me know if any adjustments are needed. Thanks! |
|
I left a comment in the linked issue. @jpeach had expressed the understanding that |
|
You are entirely correct that the However, the confusion usually stems from the transaction lifecycle in a reverse proxy setup. When a client sends a standard relative URI request (e.g., I can update the PR documentation to make this distinction clearer: that while the API operates on any valid buffer, extracting the host from a client request transaction may require waiting until post-remap if relying on the URL instead of the Host header. Let me know if you would like me to adjust the wording this way! |
|
I noticed the docs build failed with Could this be a pre-existing issue in the docs build environment, or should I change the RST syntax in the note? I can simplify it to use plain monospace text instead of Also, I've responded to the technical question on #5742 about whether |
|
Hi @JosiahWI and @bryancall, I see that #5742 has been closed as not planned. I understand that the premise of my docs change may no longer align with the project's understanding of the API. I'm happy to close this PR if it's no longer needed. Thank you for taking the time to review it. |
|
@mmustafasenoglu Thank you for explaining the situation with the |
|
@mmustafasenoglu A quick scan of our built-in plugins shows that we use Where I think we should go with this, if you are up for it:
It would be good for both the |
…se TSHttpHdrHostGet - TSHttpHdrUrlGet: add note that URL components may not be available at early hooks, recommend TSHttpHdrHostGet for reliable host retrieval - TSUrlHostGet: add call condition note (TS_HTTP_POST_REMAP_HOOK onwards) and cross-reference to TSHttpHdrHostGet - TSHttpHdrHostGet: add cross-references to TSHttpHdrUrlGet and TSUrlHostGet - redirect_1: replace TSHttpHdrUrlGet+TSUrlHostGet with TSHttpHdrHostGet which works correctly at TS_HTTP_READ_REQUEST_HDR_HOOK
|
The docs build failed due to the following warnings: /home/jenkins/workspace/Github_Builds/docs/src/doc/developer-guide/api/functions/TSHttpHdrHostGet.en.rst:48: WARNING: c:macro reference target not found: TS_HTTP_READ_REQUEST_HDR_HOOK [ref.macro] |
|
I've fixed the docs build error by replacing all
This should resolve the Thank you! |
|
There's a new docs warning:
|
|
It looks like You can run the formatter with the |
- Restored accidentally emptied TSHttpHdrHostGet.en.rst - Added call condition note explaining TSHttpHdrHostGet vs TSUrlHostGet - Added See Also cross-references to related APIs Fixes docs build warning reported by JosiahWI
|
Hi @JosiahWI, I've fixed the docs build issue. The TSHttpHdrHostGet.en.rst file had been accidentally emptied - I've restored the full content with:
The build warnings should now be resolved. Could you please re-trigger CI? Thanks! |
JosiahWI
left a comment
There was a problem hiding this comment.
Changes look good! The only issue is the missing newline at the end of redirect_1.cc.
|
Hi @JosiahWI, I've cleaned up the PR:
The diff is now minimal — only the intended doc notes and the example plugin fix. Could you take another look? Thanks! |
JosiahWI
left a comment
There was a problem hiding this comment.
Everything looks good now. Thank you for working on this!
* docs: add call condition note for TSUrlHostGet Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK. For earlier hooks like TS_HTTP_READ_REQUEST_HDR_HOOK, TSHttpHdrHostGet should be used instead. Fixes apache#5742 * docs: fix unknown interpreted text role data Replace :data: with double backticks for hook names, consistent with the rest of the documentation. Fixes apache#5742 * docs: trigger CI for TSUrlHostGet call condition note * docs: document TSHttpHdrUrlGet hook availability, fix redirect_1 to use TSHttpHdrHostGet - TSHttpHdrUrlGet: add note that URL components may not be available at early hooks, recommend TSHttpHdrHostGet for reliable host retrieval - TSUrlHostGet: add call condition note (TS_HTTP_POST_REMAP_HOOK onwards) and cross-reference to TSHttpHdrHostGet - TSHttpHdrHostGet: add cross-references to TSHttpHdrUrlGet and TSUrlHostGet - redirect_1: replace TSHttpHdrUrlGet+TSUrlHostGet with TSHttpHdrHostGet which works correctly at TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix: restore TSHttpHdrHostGet docs content and add cross-references - Restored accidentally emptied TSHttpHdrHostGet.en.rst - Added call condition note explaining TSHttpHdrHostGet vs TSUrlHostGet - Added See Also cross-references to related APIs Fixes docs build warning reported by JosiahWI * fix: add trailing newline to redirect_1.cc * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header in redirect_1.cc * fix: restore license header and fix broken string literal * fix: restore blank line after title underline * fix: add blank line between title and Synopsis * fix: apply code changes on top of upstream cleanly * fix: restore redirect_1.cc with correct API changes * fix: restore redirect_1.cc with correct TSHttpHdrHostGet usage * fix: apply TSHttpHdrHostGet changes cleanly on upstream * fix: restore blank line between title and Synopsis --------- Co-authored-by: Mustafa Senoglu <mustafa@senoglu.local>
* docs: add call condition note for TSUrlHostGet Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK. For earlier hooks like TS_HTTP_READ_REQUEST_HDR_HOOK, TSHttpHdrHostGet should be used instead. Fixes apache#5742 * docs: fix unknown interpreted text role data Replace :data: with double backticks for hook names, consistent with the rest of the documentation. Fixes apache#5742 * docs: trigger CI for TSUrlHostGet call condition note * docs: document TSHttpHdrUrlGet hook availability, fix redirect_1 to use TSHttpHdrHostGet - TSHttpHdrUrlGet: add note that URL components may not be available at early hooks, recommend TSHttpHdrHostGet for reliable host retrieval - TSUrlHostGet: add call condition note (TS_HTTP_POST_REMAP_HOOK onwards) and cross-reference to TSHttpHdrHostGet - TSHttpHdrHostGet: add cross-references to TSHttpHdrUrlGet and TSUrlHostGet - redirect_1: replace TSHttpHdrUrlGet+TSUrlHostGet with TSHttpHdrHostGet which works correctly at TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix: restore TSHttpHdrHostGet docs content and add cross-references - Restored accidentally emptied TSHttpHdrHostGet.en.rst - Added call condition note explaining TSHttpHdrHostGet vs TSUrlHostGet - Added See Also cross-references to related APIs Fixes docs build warning reported by JosiahWI * fix: add trailing newline to redirect_1.cc * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header in redirect_1.cc * fix: restore license header and fix broken string literal * fix: restore blank line after title underline * fix: add blank line between title and Synopsis * fix: apply code changes on top of upstream cleanly * fix: restore redirect_1.cc with correct API changes * fix: restore redirect_1.cc with correct TSHttpHdrHostGet usage * fix: apply TSHttpHdrHostGet changes cleanly on upstream * fix: restore blank line between title and Synopsis --------- Co-authored-by: Mustafa Senoglu <mustafa@senoglu.local> (cherry picked from commit 16c434c)
Summary
Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK.
Changes
Why
Without this note, developers may try to use TSUrlHostGet in hook phases where it won't work correctly, leading to unexpected behavior.
Fixes #5742