Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Strictly follow Content-Length header ABNF rule #10144

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Strictly follow Content-Length header ABNF rule #10144

merged 1 commit into from
Aug 7, 2023

Conversation

masaori335
Copy link
Contributor

@masaori335 masaori335 commented Aug 3, 2023 •
edited
Loading

Address #10137.

RFC 9110 8.6. Content-Length

Content-Length = 1*DIGIT

If the value filed is empty, it should be handled as a malformed header.

@masaori335 masaori335 added the HTTP label Aug 3, 2023
@masaori335 masaori335 added this to the 10.0.0 milestone Aug 3, 2023
@masaori335 masaori335 self-assigned this Aug 3, 2023
jpeach
jpeach reviewed Aug 3, 2023
View reviewed changes
proxy/hdrs/HTTP.cc Outdated Show resolved Hide resolved
@masaori335
Strictly follow Content-Length header ABNF rule
b30fece 
- Replace content-length value type from const char * to std::string_view
jpeach
jpeach approved these changes Aug 7, 2023
View reviewed changes
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Would you mind making a follow-up PR to remove the non-sotringview version of value_get?

@masaori335
Copy link
Contributor Author

I'll file an issue to deprecate the value_get at least. It'll be a big change because the function is widely used.

@masaori335 masaori335 merged commit f9aa1e0 into apache:master Aug 7, 2023
@masaori335 masaori335 mentioned this pull request Aug 7, 2023
Open
@zwoop
Copy link
Contributor

zwoop commented Aug 10, 2023

@masaori335 This has a small merge conflict for 9.2.x, can you make a branch PR please?

masaori335 added a commit to masaori335/trafficserver that referenced this pull request Aug 13, 2023
@masaori335
Strictly follow Content-Length header ABNF rule (apache#10144)
54b4ddf 
- Replace content-length value type from const char * to std::string_view

(cherry picked from commit f9aa1e0)

Conflicts:
	proxy/hdrs/unit_tests/test_Hdrs.cc
@masaori335 masaori335 mentioned this pull request Aug 13, 2023
Merged
zwoop pushed a commit that referenced this pull request Aug 14, 2023
@masaori335
Strictly follow Content-Length header ABNF rule (#10144) (#10176)
55f4755 
- Replace content-length value type from const char * to std::string_view

(cherry picked from commit f9aa1e0)

Conflicts:
	proxy/hdrs/unit_tests/test_Hdrs.cc
masaori335 added a commit to masaori335/trafficserver that referenced this pull request Sep 26, 2023
@masaori335
Strictly follow Content-Length header ABNF rule (apache#10144) (apach…
478f5ff 
…e#10176)

- Replace content-length value type from const char * to std::string_view

(cherry picked from commit f9aa1e0)

Conflicts:
	proxy/hdrs/unit_tests/test_Hdrs.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpeach jpeach jpeach approved these changes

Assignees

@masaori335 masaori335

Labels
HTTP
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@masaori335 @zwoop @jpeach