Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#8551 closed (fixed)

Add REMOTE_ADDR: '127.0.0.1' to TestClient environment by default

Reported by: Michael Schurter Owned by: Eric Holscher
Component: Testing framework Version: dev
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The fake environment django.test.client.Client sets up is missing REMOTE_ADDR. It could be very safely and accurately defaulted to localhost (127.0.0.1).

Luckily this issue is very easy to work around when instantiating the test Client:

c = Client(REMOTE_ADDR='127.0.0.1')

Attachments (1)

django-remote_addr-test.patch (498 bytes ) - added by Michael Schurter 16 years ago.

Download all attachments as: .zip

Change History (9)

by Michael Schurter, 16 years ago

comment:1 by Chris Beaven, 16 years ago

Triage Stage: UnreviewedReady for checkin

It is a pretty simple fix and I can't see any potential bad side-effects. In fact, since RFC3875 says that "The REMOTE_ADDR variable MUST be set to the network address of the client sending the request to the server." it seems fair enough to set it in the test client for a reliable test environment.

comment:2 by Eric Holscher, 16 years ago

Just ran into this as well. I fixed it by doing the suggested c = Client(REMOTE_ADDR='127.0.0.1') and it works fine. Not a huge deal, but I agree that I don't think it'll hurt anything, and seems like a sane default.

comment:3 by Eric Holscher, 16 years ago

Owner: changed from nobody to Eric Holscher

comment:4 by Rob Hudson <treborhudson@…>, 16 years ago

I just helped someone on #django IRC who was getting hung up on this as well. Seems like a quick fix that would save testers some headache.

comment:5 by Russell Keith-Magee, 16 years ago

I agree that we should do something here. The reason I haven't checked anything in is that my initial thoughts snagged on an issue, and I haven't had a chance to come back to it confirm whether its actually a problem, or if I'm overthinking things.

The point I got snagged on is this: 127.0.0.1 isn't a particularly "remote" REMOTE_ADDR. If you're testing for behavior that will be observed remotely, there could be edge cases that won't be exercised if your test case is inherently local. However, a completely fake IP (e.g., 12.34.56.78) isn't really a good idea either, since it isn't connected to anything in the real world.

However, I am fully willing to admit that this may be a delusion of my deranged mind. In this case, I just need someone to sit me down with a warm cup of cocoa, give me my blanky and say "there there" while rubbing my forehead and telling me that the bad man will go away.

comment:6 by Malcolm Tredinnick, 16 years ago

I think you need to take your blanky and go dip it in your cocoa for a bit. 127.0.0.1 should behave the same as any other IP address, at least for our purposes. It's "just" an IP address: it uses the same network stack and everything (except its routing goes to the local machine). It also happens to be the correct value.

Using a valid IP address makes sense and the 127.0.0.1 address is a reasonable guess. Determining if the machine has a non-local IP address and using that, if so, would also be valid, but a lot of extra work. Only small problem is that 127.0.0.1 is not guaranteed to be the loopback address (I've seen at least one machine in production -- at an ISP -- that used another number for reasons I didn't quite understand). IPv6-only networks also use a different string form there, too, obviously (once again, showing that using phrases like "safely and accurately" and "easily" in a bug description are warning signs). However, determining the loopback address dynamically is fairly tricky (relies on networking being set up correctly in a number of easy-to-get-wrong ways on the machine in question), so I'd vote for punting on being right 100% of the time for now and just go for 98% of the time plus an "XXX" or "FIXME" note about the other 2%.

comment:7 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

(In [9847]) Fixed #8551 -- Added REMOTE_ADDR to the default Test Client environment. Thanks to schmichael for the report and patch.

comment:8 by Russell Keith-Magee, 16 years ago

(In [9848]) [1.0.X] Fixed #8551 -- Added REMOTE_ADDR to the default Test Client environment. Thanks to schmichael for the report and patch.

Merge of r9847 from trunk.

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