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

environment case-insensitive warning painful #372

Closed
kberry opened this issue Feb 21, 2021 · 3 comments · Fixed by #383
Closed

environment case-insensitive warning painful #372

kberry opened this issue Feb 21, 2021 · 3 comments · Fixed by #383

Comments

@kberry
Copy link

kberry commented Feb 21, 2021

As of LWP::UserAgent 6.52, and perhaps somewhat earlier, it started to complain about case-insensitive "clashes" of environment variables when env_proxy was set. Even for environment variables that have nothing to do with LWP, and even when the environment is case sensitive (e.g., Unix :). This is frustrating, since it newly requires "cleaning" things which, it seems to me, LWP should not be complaining about. For example, say we have the trivial tryua.pl:

use LWP; my $ua = LWP::UserAgent->new( env_proxy => 1 );

and then run

env foo=1 FOO=2 perl tryua.pl

There is output:

Environment contains multiple differing definitions for 'foo'.                  
Using value from 'FOO' (2) and ignoring 'foo' (1) at /usr/local/lib/perl5/site_\
perl/5.32.1/LWP/UserAgent.pm line 1117.        

(Naturally, in practice the issue is not with explicitly-specified envvars like this, but case-varying envvars that I have set in my environment for reasons unrelated to LWP.)

At least for me, tt would be nice if either LWP wouldn't complain on case-sensitive systems, or wouldn't complain about envvars that it doesn't ever read, or at least had an option to suppress these checks. Thanks for considering.

@oalders
Copy link
Member

oalders commented Feb 21, 2021

That sounds like some unintended side effects of #355

Those changes are specifically made to clear up any ambiguity around having both http_proxy and HTTP_PROXY env vars set, so switching off the checks where the env vars are case sensitive would defeat part of that change set.

I think we need to restrict this check to env vars that we care about it and not warn in other cases. Pull request certainly welcome. :)

@kberry
Copy link
Author

kberry commented Feb 22, 2021

I'll append a simple diff for your consideration, essentially duplicating the "we don't care about this envvar" tests from the bottom of the loop. I can't easily generate a PR, sorry. Also, the logic of that for loop is not exactly clear to me (sorry again), so I expect you'll want to adjust the change in some wa or another. Please consider this contribution trivial/uncopyrightable/public domain/do as you like :). Thanks for considering.

--- ORIG/UserAgent.pm   2021-01-07 22:21:10.000000000 +0100
+++ UserAgent.pm        2021-02-22 23:52:37.864725750 +0100
@@ -1114,2 +1114,6 @@
         if (my $from_key= $seen{$k}) {
+            # Do not complain about envvars that we never look at.
+            next unless $k =~ /^(.*)_proxy$/;
+            next unless $k =~ /^$URI::scheme_re\z/;
+            next unless LWP::Protocol::implementor($k);
             warn "Environment contains multiple differing definitions for '$k'.\n".

@oalders
Copy link
Member

oalders commented Feb 22, 2021

Thanks @kberry!

eserte added a commit to eserte/libwww-perl that referenced this issue Aug 11, 2021
This may help if changes to fix libwww-perl#372 are done.
eserte added a commit to eserte/libwww-perl that referenced this issue Aug 11, 2021
This may help if changes to fix libwww-perl#372 are done.
eserte added a commit to eserte/libwww-perl that referenced this issue Aug 11, 2021
Before doing changes to fix libwww-perl#372 it is good to have a
complete test coverage of env_proxy().
eserte added a commit to eserte/libwww-perl that referenced this issue Aug 11, 2021
Before doing changes to fix libwww-perl#372 it is good to have a
complete test coverage of env_proxy().
eserte added a commit to eserte/libwww-perl that referenced this issue Aug 11, 2021
Before doing changes to fix libwww-perl#372 it is good to have a
complete test coverage of env_proxy().
oalders pushed a commit that referenced this issue Aug 11, 2021
Before doing changes to fix #372 it is good to have a
complete test coverage of env_proxy().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants