Opened 6 years ago

Closed 6 years ago

#4918 defect closed fixed (fixed)

web.http.Request.setHost ignores the port when setting the host header

Reported by: Jason Michalski Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight, Jason Michalski Branch:
Author:

Description

While working on 3845 (which ended up looking like a dup of 2648) I noticed that web.http.Request.setHost is ignoring the port number when setting the host header.

Attachments (2)

.0001-web.http.Request.setHost-now-sets-the-port-in-the-ho.patch.swp (16.0 KB) - added by Jason Michalski 6 years ago.
0001-web.http.Request.setHost-now-sets-the-port-in-the-ho.patch (3.7 KB) - added by Jason Michalski 6 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 6 years ago by Jason Michalski

Cc: Jason Michalski added
Keywords: review added
Owner: Jason Michalski deleted

Ignore the first attachment which is the wrong file.

comment:3 Changed 6 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jean-Paul Calderone

Thank you for the patch, armooo. This looks great. One minor point, we're preferring assertEquals over assertEqual for new tests, and we use camelCase instead of under_scores for things like the host_header local in setHost. Those are trivial though, I'll adjust the patch and then apply it. Thanks again!

comment:4 Changed 6 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [30876]) Apply patch to cause Request.setHost to set a header which includes port information if appropriate.

Author: armooo Reviewer: exarkun Fixes: #4918

If the port passed to Request.setHost isn't the default for the scheme, include it in the header added to Request.requestHeaders to more closely imitate the header which would be present if the client were really issuing requests to the specified host and port.

Note: See TracTickets for help on using tickets.