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.

Sign up for GitHub

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

Jump to bottom

let LJ::OpenID use the paranoid user agent on dev servers #1468

Closed
kareila opened this issue Jun 29, 2015 · 5 comments
Closed

let LJ::OpenID use the paranoid user agent on dev servers #1468

kareila opened this issue Jun 29, 2015 · 5 comments
Assignees
@kareila
Labels
status: claimed status: untriaged

Comments

@kareila
Copy link
Member

kareila commented Jun 29, 2015

Related to #1418: when we switched modules from LWPx::ParanoidAgent to LWP::UserAgent::Paranoid, there were some API changes which made it to the live site undetected because LJ::OpenID does not use the paranoid user agent on dev servers. The error was fixed in #1460, but in the future, I believe such heartache could be avoided if we removed the dev server special casing. The split dates from LJ days and I haven't seen any good arguments in favor of keeping it.
@kareila kareila self-assigned this Jun 29, 2015
@alierak
Copy link
Member

alierak commented Jun 29, 2015

I think the only argument in favor might be that in a dev environment, you might actually want to access a URL that resolves to a private IP, if that's where you happen to be hosting a test site.

On Jun 29, 2015, at 3:28 PM, Jen notifications@github.com wrote:

Related to #1418: when we switched modules from LWPx::ParanoidAgent to LWP::UserAgent::Paranoid, there were some API changes which made it to the live site undetected because LJ::OpenID does not use the paranoid user agent on dev servers. The error was fixed in #1460, but in the future, I believe such heartache could be avoided if we removed the dev server special casing. The split dates from LJ days and I haven't seen any good arguments in favor of keeping it.

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

@kareila
Copy link
Member Author

kareila commented Jun 29, 2015

We might want to make that a user-configurable option instead of the default, then.

@afuna
Copy link
Member

afuna commented Jun 30, 2015

Hmm that's a good point -- but I think it makes sense to have it be something you turn on (or even just something we add when / if we ever need it) than having it magically happen behind the scenes.

@kareila
Copy link
Member Author

kareila commented Jun 30, 2015

We could just change the existing check for IS_DEV_SERVER to check for a config option instead, maybe call it SKIP_PARANOID_UA?

@afuna
Copy link
Member

afuna commented Jun 30, 2015

I would suggest USERAGENT instead of UA but otherwise I'm fine with those

On Jul 1, 2015, at 05:44, Jen notifications@github.com wrote:

We could just change the existing check for IS_DEV_SERVER to check for a config option instead, maybe call it SKIP_PARANOID_UA?

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

kareila added a commit to kareila/dreamwidth that referenced this issue Jul 1, 2015
@kareila
[dreamwidth#1468] check for SKIP_PARANOID_USERAGENT instead of IS_DEV…
cb518f2 
…_SERVER

For certain LJ::OpenID functions that require a UserAgent object,
use LWP::UserAgent::Paranoid instead of the default LWP::UserAgent,
unless $LJ::SKIP_PARANOID_USERAGENT is enabled in the site config.

Fixes dreamwidth#1468.
@kareila kareila mentioned this issue Jul 1, 2015
Closed
kareila added a commit to kareila/dreamwidth that referenced this issue Jul 1, 2015
@kareila
[dreamwidth#1468] let LJ::OpenID always use the paranoid useragent
f8915a7 
This removes the IS_DEV_SERVER special casing that resulted in the
paranoid useragent only being used in production for some functions
of LJ::OpenID.

The bypass for the blocked_hosts check is left intact as a comment,
in case it is needed for testing purposes at a later date.

Fixes dreamwidth#1468.
@kareila kareila mentioned this issue Jul 1, 2015
Merged
zorkian added a commit that referenced this issue Jul 1, 2015
@zorkian
Merge pull request #1476 from kareila/1468-paranoid_always
0491e2c 
[#1468] let LJ::OpenID always use the paranoid useragent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kareila kareila

Labels
status: claimed status: untriaged
Milestone
No milestone
Development

No branches or pull requests
4 participants
@afuna @kareila @alierak @robodw-issues