Opened 4 years ago

Closed 4 years ago

#4918 defect closed fixed (fixed)

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

Reported by: armooo Owned by: exarkun
Priority: normal Milestone:
Component: web Keywords:
Cc: jknight, armooo@… Branch:
Author: Launchpad Bug:

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.

Change History (6)

comment:1 Changed 4 years ago by DefaultCC Plugin

  • Cc jknight added

comment:2 Changed 4 years ago by armooo

  • Cc armooo@… added
  • Keywords review added
  • Owner armooo deleted

Ignore the first attachment which is the wrong file.

comment:3 Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner set to exarkun

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 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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.