Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$r->connection->remote_ip removed on Apache 2.4 #923

Closed
anall opened this issue Aug 10, 2014 · 23 comments

Comments

Projects
None yet
6 participants
@anall
Copy link
Member

commented Aug 10, 2014

Apache 2.4 / mod_perl 2.4 have removed $r->connection->remote_ip -- Replacements are either $r->connection->client_ip ( closest load balancer, I think ) and $r->useragent_ip ( actual user IP -- may not be secure depending on how the X-Forwarded-For headers are trusted in upstream proxies ) seem to be the possible replacements.

$r->useragent_ip seems to be the correct replacement, however I am unsure if that works on pre-2.4 and do not have easy access to check.

@anall anall changed the title `$r->connection->remote_ip` removed on Apache 2.4 $r->connection->remote_ip removed on Apache 2.4 Aug 10, 2014

@anall

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2014

##effort: average
##severity: minor

@kareila

This comment has been minimized.

Copy link
Member

commented Aug 11, 2014

##effort: average
##severity: minor

@anall

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2014

##effort: average ##severity: minor

@anall

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2014

##effort: lower
##severity: minor

@kareila

This comment has been minimized.

Copy link
Member

commented Aug 11, 2014

claiming?

@anall

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2014

##effort: average
##severity: minor

Bah spammy issue is spammy

@pauamma

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2014

I don't find a mention of useragent_ip in any of

  • perldoc Apache2::Request
  • perldoc Apache2::RequestRec
  • perldoc Apache2::Connection

That's for both Apache/2.2.14 (Ubuntu) mod_apreq2-20051231/2.6.0 mod_perl/2.0.4 Perl/v5.10.1 and Apache/2.2.22 (Ubuntu) mod_apreq2-20090110/2.8.0 mod_perl/2.0.5 Perl/v5.14.2, as logged to the Apache errorlog on start.

@afuna

This comment has been minimized.

Copy link
Member

commented Feb 4, 2015

@zorkian -- so you're aware of this issue.

@anall

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2015

Any thoughts on this? This is blocking me from easily using my development environment without juggling temporary changes and risking committing unintended changes.

@zorkian

This comment has been minimized.

Copy link
Member

commented Feb 14, 2015

Oh yeah. I have a crappy patch in my new 2.4 hack, I am using client_ip. I looked at the source code between 2.2 and 2.4 and they just renamed remote_ip to client_ip.

Generally we can't use useragent_ip unless we want to get rid of the ability to turn off trusting X-Forwarded-For. Which, honestly, I'd be OK with. That would simplify a bunch of our code since we could just say "we trust, put a proxy in front".

@anall

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2015

I was under the impression that useragent_ip followed Apache's X-Forwarded-For trust rules

@zorkian

This comment has been minimized.

Copy link
Member

commented Feb 14, 2015

It does. But since we have all those trust rules built into our own code, we don't want the two to conflict, and I'd rather not have to configure Apache correctly on top of configuring DW correctly.

ETA: Also, I still want us to get rid of Apache. ;)

@anall

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2015

I really want to kill Apache in the face too! Possibly once we've fully killed off BML?

A thought for this, as we may find other random issues with 2.4 -- create DW::Request::Apache2_4 as a subclass of DW::Request::Apache2 and override whatever relevant ( v.s. adding various code such as

return $conn->can('client_ip') ? $conn->client_ip : $conn->remote_ip;
@zorkian

This comment has been minimized.

Copy link
Member

commented Feb 19, 2015

Well, I think the code is only used in three places, so the ->can syntax is not TOO heinous. It's also in BML/Apache::LiveJournal/DW::Request so it's unlikely people beyond you and I will be touching it, so I think that's OK?

(If we had many more uses, I'd be A++ to a subclass. But since it's just three, I'm not sure it seems worth it?)

@afuna

This comment has been minimized.

Copy link
Member

commented Feb 28, 2015

I feel like this kind of thing has the potential to have things tacked
onto it as time goes by and we find new things. But why don’t we just
mandate a minimum of apache 2.4 after a certain point?

@zorkian

This comment has been minimized.

Copy link
Member

commented Mar 4, 2015

Sure. We can update production to 14.04 if we wanted to do that as part of a code push. I'd just have to shut down some web servers, upgrade them, then we would do the code push.

So, why don't we just update DW to be 2.4 required? It's not a hard upgrade.

@pauamma

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2015

And upgrade the Dreamhack hosts as well?

@anall

This comment has been minimized.

Copy link
Member Author

commented May 6, 2015

Can I go ahead and put in a stopgap pull request for using return $conn->can('client_ip') ? $conn->client_ip : $conn->remote_ip;

This prevents me from easily doing any work on my dev env.

@zorkian

This comment has been minimized.

Copy link
Member

commented May 6, 2015

Can't you update to 14.04? That's what I did?

@anall

This comment has been minimized.

Copy link
Member Author

commented May 6, 2015

Am I missing something here? I opened this issue when I reinstalled my dev env with 14.04?

$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu 14.04.1 LTS"

$ git show 
commit 510d7911d077f0096f8e09e845d80be14b76f356
Merge: 995681b c097e65
Author: Mark Smith <mark@qq.is>
Date:   Tue May 5 21:09:40 2015 -0700

Then I get this on pretty much every page:

[Error: Can't locate object method "remote_ip" via package "Apache2::Connection" at /home/dw/dw/cgi-bin/DW/Request/Apache2.pm line 173. ]
@zorkian

This comment has been minimized.

Copy link
Member

commented May 7, 2015

@fu said we should just require 14.04/2.4 and be done. I agree. So, no can.

@afuna

This comment has been minimized.

Copy link
Member

commented May 8, 2015

Pinging @Sophira (want to make sure you're aware of the soon-to-be-updated requirements)

@afuna

This comment has been minimized.

Copy link
Member

commented May 9, 2015

Heh, reading this via email is confusing. It sort of looks like you're
saying we can't go forward with this (versus no "can" / if statement)

On Thu, May 7, 2015, at 10:31 AM, Mark Smith wrote:

@fu[1] said we should just require 14.04/2.4 and be done. I agree.
So, no can.

— Reply to this email directly or view it on GitHub[2].

Links:

  1. https://github.com/fu
  2. #923 (comment)

anall added a commit to anall/dw-free that referenced this issue May 9, 2015

Use client_ip.
Note: this breaks everything for anything before Apache 2.4

Fixes dreamwidth#923

@anall anall referenced this issue May 9, 2015

Merged

Use client_ip. #1376

anall added a commit to anall/dw-free that referenced this issue May 9, 2015

Use client_ip.
Note: this breaks everything for anything before Apache 2.4

Fixes dreamwidth#923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.