moonchen commented on PR #13376:
URL: https://github.com/apache/trafficserver/pull/13376#issuecomment-5787762652

   **`validate_hdr_request_target`** — Fixed in d83aaa659c: tests the pointer 
along with the length, so it now matches the getters it replaced exactly.
   
   **`check_for_dups` comment** — The original reasoning holds. 
`mime_hdr_field_attach` searches with `field->name_get()`, which returns the 
interned WKS string whenever `m_wks_idx >= 0`, not `m_ptr_name`. So 
`mime_hdr_field_find` does take the WKS branch, and its first negative test is 
this presence bit with the same mask (`hdrtoken_str_masks[i]` is built from 
`wks_info.mask`). Reworded the comment in f58829fbc7 to say that directly.
   
   **Tail-append comment** — Rewritten in 0fe3be7ae8; it now says the pointer 
check is required because `mime_field_create_for_name()` can reuse an older 
slot.
   
   **URL diagnostics** — I agree that a rejection-only scan would preserve the 
vectorizable loop. I'd still prefer to leave these diagnostics removed: 
identifying the offending byte doesn't provide enough troubleshooting value to 
justify the additional scan and logging logic in this frequently executed 
validator.
   
   **ASAN** — `test_proxy_hdrs` is clean under ASAN. Autests under ASAN with 
leak detection off: `h2_origin_trailers_h1`, `http2`, 
`http2_crlf_header_validation`, `chunked_encoding`, `chunked_encoding_h2`, 
`chunk_trailer_bare_lf`, `chunked_not_last`, `bad_chunked_encoding`. There were 
no AddressSanitizer reports. One `http2` case failed locally only because its 
curl command uses `|&`, which dash rejects.
   
   **Non-blocking:**
   - The URL test now uses the old `isspace`/`isprint` pair as its reference.
   - The parity test now sweeps the whole WKS table in three case forms.
   - `hdrtoken_tokenize_prehashed` now asserts `!hdrtoken_is_wks(string)`.
   - The `URL.cc` comment block is removed.
   - `MIME_HDR_SANITY_CHECK` is compiled out in every build 
(`ENABLE_MIME_SANITY_CHECK` is never defined), so adding it to the fast path 
would be a no-op. Left as is.
   - Left the `INT_MAX` guard out of the commit message; a multi-gigabyte 
header line isn't worth the noise.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to