Opened 4 years ago

Closed 4 years ago

#17067 closed Bug (fixed)

REMOTE_ADDR should be set to 127.0.0.1 in the test client environment

Reported by: jsdalton Owned by: nobody
Component: Testing framework Version: 1.3
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Per #8551, the REMOTE_ADDR should be set to 127.0.0.1 in the test client environment by default. However it appears that [16933] for #16705 inadvertently removed it and this variable is no longer available in the test client environment.

Attachments (1)

restore_remote_addr_to_test_client_env.diff (1.6 KB) - added by jsdalton 4 years ago.

Download all attachments as: .zip

Change History (4)

Changed 4 years ago by jsdalton

comment:1 Changed 4 years ago by jsdalton

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Added a patch that restores the environment variable. I also added tests to prevent this kind of regression in the future.

comment:2 Changed 4 years ago by aaugustin

  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

Indeed, it's a regression => release blocker.

The root of the problem is that #8551 and #16705 have been resolved with different mindsets: "do something reasonable for most cases" vs. "follow the reference documentation strictly".


I'd prefer following a solid reference before adding / removing more variables to / from the default environment of the test client. However, I didn't find a good one.

The current reference is PEP 3333; I admit it isn't totally realistic. Looking at RFC 3875 (CGI 1.1, http://www.ietf.org/rfc/rfc3875), most of the meta-variables defined there are irrelevant, so it isn't a good starting point either.

Thus, I think that the best fix is to put back REMOTE_ADDR in the default environment, with a comment saying "not defined by PEP 3333, but provided by most webservers, see #8551"

comment:3 Changed 4 years ago by aaugustin

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

In [17010]:

Fixed #17067 -- reverted some backwards-incompatible changes from r16933 and added tests.

Note: See TracTickets for help on using tickets.
Back to Top