Repository navigation
Keep an application name a header cannot carry out of the User-Agent - #32
Open
dmccoystephenson wants to merge 1 commit into
Open
dmccoystephenson wants to merge 1 commit into
dmccoystephenson wants to merge 1 commit into
Conversation
http.client refuses a header outside Latin-1 or with a bare line break, so a program named outside that range lost every event at DEBUG. Those characters become ? in the header; the JSON body keeps the exact name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Self-review rubric (head
Findings:
Merge hold: this PR changes a header in This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
applicationname outside Latin-1, or one containing a line break, lost every event:http.clientrefused theUser-Agentvalue and the failure was logged only atDEBUG. A new private helper,_user_agent, replaces any character outside printable Latin-1 with?in that header.applicationin the JSON body stays exact. Printable Latin-1 names keep the header they sent before, andZürichis asserted to prove it.User-Agentshape and the?rule are added to the README's wire format section, where the header was not documented.Closes #30
Test plan
python -m unittestran 69 tests, OK, on Python 3.8.10 (68 before this change)test_an_application_name_a_header_cannot_carry_still_delivers_its_eventscovers Japanese, an emoji, a line break and a Latin-1 name1 != 0 : 'ゲーム' lost its event; with it restored, the suite passedtest (3.8),test (3.10)andtest (3.12)green on the PR headTrace server and Java
This is a header change in
_send, which is on the dev loop's do-not-auto-merge list, so this PR is left for owner review. No body field changes. Two kinds of name see a differentUser-Agent: names that previously sent nothing now send one, and names holding a tab or other control character, whichhttp.clientlet through, now show?for it. Printable Latin-1 names are unaffected.Whether trace-client-java needs the same change is not verified: its source could not be read from this session. Its README does not describe the
User-Agent.Issues not picked
_sendand has its own review needs, so it was kept out of this PR.This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson
🤖 Generated with Claude Code