Internal browser and ip_address() inconsistent

The IP could be set to something in a similar manor to the db prefix. What should it be set to?

Marking accordingly.

Not sure really, 127.0.0.1 maybe?

just pass on the real ip_address

This would be very simple to fix, just check for SimpleTest user-agent then set IP to x if it doesn't come through.

I can't test this because it passes on my machine.

Bump, we need to pick an IP address and get this fixed. I would second 127.0.0.1.

@boombatower - do you mean in the test itself or somewhere more central?

More central, but if this is the only place where it occurs perhaps just in the test would be fine.

For consistency and to ensure this doesn't come up again it should probably be more central.

Since this appears to be a curl bug, here's a patch to simply roll back that part of the test - we can then mark this postponed against a curl issue (which I believe chx is arranging).

General consensus has been to keep test passes at 100% (once they all pass), and not commit tests that we know are going to fail - this one kinda breaks that rule so I think it's acceptable to remove.

Since I can't reproduce this anymore myself, marking as won't fix. Can re-open if someone else does.

Sorry, I can indeed reproduce this.

This patch makes sense, but I think we need to contact the cURL developers and inform them of this issue.

Once resolved this needs to be reverted.

Once this is committed lets mark this as postponed.

Otherwise we can just ignore the environments that this doesn't pass on. :)

Ideally we want all tests passing on any environments they can be run on - otherwise we'll get bogus test failures while people are trying to write patches or just showing up in the bug queue over and over again. Another option might be to wrap that part of the tests in an if, to ensure ip_address() returns something before doing the assertion - but ideally it can be dealt with on the cURL end.

I've decided to comment out this test instead of deleting it. I've also added a note to explain why it was commented out.

I'm marking this issue 'code needs work'. I think we should mark it 'fixed' once we have reported this bug to the cURL maintainers.

curl version info on Mac OS X Leopard:

(Note that I did NOT check if tests are working or failing for me.)

Isn't this probably a PHP and/or Apache bug rather than a cURL bug?

I can't understand why the client (here cURL) could have any impact on the server's ability to detect it's IP address. After all, this is done at the TCP layer, not at the HTTP one. We need more information on this.

What is the return of $_SERVER['REMOTE_ADDR'] in the case it fails?

Ok, here is a fix for that.

Tested in the both cases I could think of (the hostname specified in $base_url points to 127.0.0.1 and that hostname points to the IP of an external interface), tests pass.

About the debate on whether we should "teach" ip_address() to fake it's return when we are in a test request: I think we shouldn't, because there are no needs to at that point.

Works for me :)

Doesn't work for me yet.

In the test, $parsed['host'] is 'localhost' and the IP in $edit['ip'] is '127.0.0.1'.

Given that, I've put the following two lines in system_ip_blocking_form_validate():

This is what ends up in the log file when running the tests:

It looks like ip_address() doesn't return a proper IP address (when running simpletest)?

Hmm. I'll try again on my system where it seemed to fix the problem, but looks like we might have to leave this postponed against cURL still.

@Dries#21: ip_address() can't be wrong, it just report the IP address of the client as seen by the server. Except that here it's the address of the IPv6 loopback interface.

What's wrong is my code that tries to guess what the IP address of the client *as seen by the server* could possibly be if we call the server from the same box as the client.

Can you paste your /etc/hosts? I guess there is a localhost ::1 there.

Damien, you're right! Here is my /etc/hosts:

So this sounds like it's a configuration problem then? We should probably add this test back in.

The last submitted patch failed testing.