inets: Remove default empty TE header from HTTP requests #10120

iamyojimbo · 2025-08-19T15:42:21Z

The httpc client was unconditionally setting the TE header to an empty string on all HTTP/1.1 requests when no TE header was explicitly provided. This behavior violated RFC 2616 and caused compatibility issues with some proxy servers that would drop requests containing empty TE headers.

RFC 2616 does not require a TE header to be present, and when absent, no transfer encoding expectations should be assumed. This change removes the automatic addition of empty TE headers, allowing httpc to send cleaner HTTP requests that better comply with the standard.

Fixes GH-10065

CLAassistant · 2025-08-19T15:42:28Z

All committers have signed the CLA.

github-actions · 2025-08-19T15:43:09Z

CT Test Results

2 files 22 suites 10m 4s ⏱️
360 tests 353 ✅ 7 💤 0 ❌
647 runs 587 ✅ 60 💤 0 ❌

Results for commit 2fab8cc.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

Copilot

Pull Request Overview

This PR removes the automatic addition of empty TE (Transfer-Encoding) headers from HTTP/1.1 requests to improve RFC 2616 compliance and resolve compatibility issues with proxy servers that drop requests containing empty TE headers.

Key changes:

Removes automatic empty TE header injection for HTTP/1.1 requests
Adds proper TE header handling that modifies the Connection header when TE is explicitly provided
Implements comprehensive test coverage for various TE header scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`lib/inets/test/httpc_SUITE.erl`	Adds extensive test cases for TE header behavior and Connection header modifications
`lib/inets/src/http_lib/http_util.erl`	Adds utility function to parse Connection header tokens
`lib/inets/src/http_client/httpc_request.erl`	Updates connection closing logic to properly parse Connection header tokens
`lib/inets/src/http_client/httpc_handler.erl`	Removes automatic empty TE header from TLS tunnel requests
`lib/inets/src/http_client/httpc.erl`	Replaces automatic TE header injection with proper TE/Connection header coordination

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lib/inets/test/httpc_SUITE.erl

lib/inets/src/http_lib/http_util.erl

lib/inets/src/http_client/httpc.erl

Whaileee · 2025-08-21T08:08:59Z

Thanks for the contribution. Change looks good to me. Please replace the strip with trim, and I think it's good to go

Whaileee · 2025-08-22T09:09:41Z

Hi, there seems to be an error in all of the added testcases, when run in group sim_http_ipv6. All tests fail with connection header "close".
Did you run ipv6 tests? Are you able to reproduce that?
Best regards,
Konrad

iamyojimbo · 2025-08-22T12:10:08Z

Hi, there seems to be an error in all of the added testcases, when run in group sim_http_ipv6. All tests fail with connection header "close". Did you run ipv6 tests? Are you able to reproduce that? Best regards, Konrad

I have not: I need to figure out how to get those running. They were skipping for me locally. I'd assumed those would be unrelated to my change, but if they were breaking, would be caught in CI.

Just so I understand: CI has passed all tests. Are the ipv6 tests not run there?

Whaileee · 2025-08-22T12:19:54Z

Ipv6 tests are also skipped in GH actions. Other machines; not visible here have these tests running and all of them, are failing there.

iamyojimbo · 2025-08-22T12:44:23Z

Got it - I will resolve. Thanks for the review!

Whaileee · 2025-08-26T11:49:29Z

Thanks for the fix. Could you rebase your changes since there is a merge conflict in httpc_SUITE?
Best regards,
Konrad

iamyojimbo · 2025-08-26T11:51:13Z

@Whaileee - please take another look - tests passing on my side. I changed ipv6 to not send per-request connection options (socket_opts), because that cases httpc to override all request connection headers with connection: close. Looks like there's some emotional baggage around that in the code:

otp/lib/inets/src/http_client/httpc_manager.erl

Lines 743 to 760 in 286418c

    
           %% Simple socket options handling (ERL-441). 
        
           %% 
        
           %% TODO: Refactor httpc to enable sending socket options in requests 
        
           %%       using persistent connections. This workaround opens a new 
        
           %%       connection for each request with non-empty socket_opts. 
        
           handle_request(Request0 = #request{socket_opts = SocketOpts}, 
        
                          State0 = #state{options = Options0}) 
        
             when is_list(SocketOpts) andalso length(SocketOpts) > 0 -> 
        
               Request = handle_cookies(generate_request_id(Request0), State0), 
        
               Options = convert_options(SocketOpts, Options0), 
        
               State = State0#state{options = Options}, 
        
               Headers = 
        
           	(Request#request.headers)#http_request_h{connection 
        
           						    = "close"}, 
        
               %% Reset socket_opts to avoid setopts failure. 
        
               start_handler(Request#request{headers = Headers, socket_opts = []}, State), 
        
               %% Do not change the state 
        
               {reply, {ok, Request#request.id}, State0};

Rather than change httpc_manager: handle_request to preserve tokens of the existing connection header, I just split out sim_http_ipv6 group to use httpc_options instead of socket_opts and left http_ipv6 as-is.

Let me know if these test pass now.

Whaileee · 2025-08-26T11:54:49Z

Whoops my bad, there wasn't a conflict (yet). Sorry!

lib/inets/test/httpc_SUITE.erl

Whaileee · 2025-08-28T07:55:01Z

Thanks! The internal tests are ran overnight so we need to wait a day for them. Change looks good to me. If the tests pass, then we are merging!

iamyojimbo · 2025-08-29T10:18:04Z

@Whaileee How'd the tests go?

Whaileee · 2025-08-29T10:22:11Z

Well, they definitely would have ran better with the testing label on 😅. I forgot to reapply that yesterday

iamyojimbo · 2025-09-01T07:12:12Z

@Whaileee Any luck?

Whaileee · 2025-09-01T08:51:26Z

For some reason the branch was still commented out of testing. I've now added it manually, so if it's not in testing now I'll start breaking stuff (just kidding). Therefore I ask for another day of patience.

iamyojimbo · 2025-09-01T10:06:59Z

No problem! 🙏

iamyojimbo · 2025-09-02T08:37:32Z

Is today the day? 🙏 I have a good feeling 😄

Whaileee · 2025-09-02T09:07:25Z

We will see. The tests on the machines with ipv6 are still running. Should be done in a few hours

Whaileee · 2025-09-02T10:05:32Z

2 out of 3 passed, I suspect in an hour we will see the rest also finish

Whaileee · 2025-09-02T12:15:38Z

3 out of 3. It's ready to be sent to the wild!
You can leave the rest for me.
Thanks for the contribution.
Best regards,
Konrad

Whaileee · 2025-09-02T13:16:31Z

I rebased it to patch-base-26 so it can be easliy backported, and removed dangling whitespaces.

The httpc client was unconditionally setting the TE header to an empty string on all HTTP/1.1 requests when no TE header was explicitly provided. This behavior violated RFC 2616 and caused compatibility issues with some proxy servers that would drop requests containing empty TE headers. RFC 2616 does not require a TE header to be present, and when absent, no transfer encoding expectations should be assumed. This change removes the automatic addition of empty TE headers, allowing httpc to send cleaner HTTP requests that better comply with the standard. Fixes erlangGH-10065

iamyojimbo · 2025-09-03T10:11:03Z

Great! Thank you! So in the next minor patch of OTP 26, 27 or 28, this will be pulled in?

Whaileee · 2025-09-03T10:16:27Z

That is true for 26 and 27, but for 28 it will be in 28.1

iamyojimbo · 2025-09-03T10:40:24Z

🙏

iamyojimbo · 2025-09-03T13:27:39Z

Just to clarify: by minor patch, do we mean ~quarterly minor patches?

Whaileee · 2025-09-03T13:53:50Z

This branch will be released with OTP 27.3.4.3 and OTP 26.2.5.15 which are planned to be released soon, as well as OTP 28.1 which will be also released soon-ish.
There are no minor patches for 26 and 27, only emergency ones which contain bugfixes for currently supported versions (last number in version)

iamyojimbo force-pushed the httpc/te-header branch 3 times, most recently from ab6adfa to f314280 Compare August 19, 2025 15:57

iamyojimbo mentioned this pull request Aug 19, 2025

httpc violates RFC 2616 for default requests #10065

Closed

IngelaAndin added the team:PS Assigned to OTP team PS label Aug 20, 2025

IngelaAndin assigned Whaileee Aug 20, 2025

Whaileee added the testing currently being tested, tag is used by OTP internal CI label Aug 20, 2025

Whaileee requested a review from Copilot August 21, 2025 08:02

Copilot AI reviewed Aug 21, 2025

View reviewed changes

lib/inets/test/httpc_SUITE.erl Outdated Show resolved Hide resolved

lib/inets/src/http_lib/http_util.erl Outdated Show resolved Hide resolved

lib/inets/src/http_client/httpc.erl Show resolved Hide resolved

iamyojimbo force-pushed the httpc/te-header branch from f314280 to 5ccc3c5 Compare August 22, 2025 09:15

iamyojimbo force-pushed the httpc/te-header branch from 5ccc3c5 to d758fbb Compare August 26, 2025 11:37

Whaileee added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Aug 26, 2025

iamyojimbo force-pushed the httpc/te-header branch from d758fbb to b3e3364 Compare August 26, 2025 11:53

Whaileee reviewed Aug 27, 2025

View reviewed changes

lib/inets/test/httpc_SUITE.erl Outdated Show resolved Hide resolved

iamyojimbo force-pushed the httpc/te-header branch from b3e3364 to 8dc0871 Compare August 27, 2025 20:13

Whaileee removed the testing currently being tested, tag is used by OTP internal CI label Aug 28, 2025

Whaileee added the testing currently being tested, tag is used by OTP internal CI label Aug 29, 2025

Whaileee previously approved these changes Sep 2, 2025

View reviewed changes

Whaileee dismissed their stale review via cd629f6 September 2, 2025 13:15

Whaileee force-pushed the httpc/te-header branch from 8dc0871 to cd629f6 Compare September 2, 2025 13:15

Whaileee previously approved these changes Sep 2, 2025

View reviewed changes

Whaileee force-pushed the httpc/te-header branch from cd629f6 to c1e39d2 Compare September 2, 2025 14:38

Whaileee dismissed their stale review via 9f4d23c September 2, 2025 15:05

Whaileee force-pushed the httpc/te-header branch 3 times, most recently from ad43f78 to 4ee1e2d Compare September 2, 2025 15:50

Whaileee force-pushed the httpc/te-header branch from 4ee1e2d to 2fab8cc Compare September 2, 2025 15:52

Whaileee approved these changes Sep 2, 2025

View reviewed changes

Whaileee merged commit b0e61aa into erlang:maint Sep 2, 2025
23 checks passed

inets: Remove default empty TE header from HTTP requests #10120

inets: Remove default empty TE header from HTTP requests #10120

Uh oh!

Conversation

iamyojimbo commented Aug 19, 2025

Uh oh!

CLAassistant commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CT Test Results

Artifacts

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Whaileee commented Aug 21, 2025

Uh oh!

Whaileee commented Aug 22, 2025

Uh oh!

iamyojimbo commented Aug 22, 2025

Uh oh!

Whaileee commented Aug 22, 2025

Uh oh!

iamyojimbo commented Aug 22, 2025

Uh oh!

Whaileee commented Aug 26, 2025

Uh oh!

iamyojimbo commented Aug 26, 2025

Uh oh!

Whaileee commented Aug 26, 2025

Uh oh!

Uh oh!

Whaileee commented Aug 28, 2025

Uh oh!

iamyojimbo commented Aug 29, 2025

Uh oh!

Whaileee commented Aug 29, 2025

Uh oh!

iamyojimbo commented Sep 1, 2025

Uh oh!

Whaileee commented Sep 1, 2025

Uh oh!

iamyojimbo commented Sep 1, 2025

Uh oh!

iamyojimbo commented Sep 2, 2025

Uh oh!

Whaileee commented Sep 2, 2025

Uh oh!

Whaileee commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Whaileee commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Whaileee commented Sep 2, 2025

Uh oh!

Uh oh!

iamyojimbo commented Sep 3, 2025

Uh oh!

Whaileee commented Sep 3, 2025

Uh oh!

iamyojimbo commented Sep 3, 2025

Uh oh!

iamyojimbo commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Whaileee commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

CLAassistant commented Aug 19, 2025 •

edited

Loading

github-actions bot commented Aug 19, 2025 •

edited

Loading

Whaileee commented Sep 2, 2025 •

edited

Loading

Whaileee commented Sep 2, 2025 •

edited

Loading

iamyojimbo commented Sep 3, 2025 •

edited

Loading

Whaileee commented Sep 3, 2025 •

edited

Loading