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

ARXIVCE-2902 use first X-Forwarded-For IP in /institutional_banner #808

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

dginev
Copy link
Contributor

@dginev dginev commented Dec 6, 2024

SSIA

@dginev dginev requested review from cbf66 and a team December 6, 2024 19:46
result = get_institution(request.remote_addr)
forwarded_ips = request.headers.getlist("X-Forwarded-For")
if len(forwarded_ips)>0:
ip = str(forwarded_ips[0]).split(',')[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the forwarded ips be space-delimited, too?

Do you need code to strip the ip string of leading and trailing spaces?

Do you need more robustness / error checking? What if forwarded_ips is None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On point 3, I double-checked, .getlist returns an empty list in the case the header isn't present.

I'll handle some more whitespaces, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. They are defined as comma-separated, so this should handle the specified syntax.

@dginev dginev force-pushed the institutional-banner-x-forwarded-for branch from f444a08 to b6fc06a Compare December 11, 2024 21:29
@cbf66 cbf66 requested a review from jonathanhyoung December 11, 2024 21:38
@dginev
Copy link
Contributor Author

dginev commented Dec 11, 2024

Rebased on develop and ensured type-checking is happy.

@cbf66 cbf66 added this pull request to the merge queue Dec 11, 2024
Merged via the queue into develop with commit 99da570 Dec 11, 2024
4 checks passed
@cbf66 cbf66 deleted the institutional-banner-x-forwarded-for branch December 11, 2024 22:08
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 this pull request may close these issues.

3 participants