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

fix(remix): Attempt to extract user IP from request headers. #6263

Merged
merged 3 commits into from Nov 24, 2022

Conversation

onurtemizkan
Copy link
Collaborator

@onurtemizkan onurtemizkan commented Nov 22, 2022

Ref: #6139 - #6139 (comment)

Remix requests don't contain client IP addresses in req.ip or req.socket.*.
To extract them, we need to iterate over a set of request headers that may contain that info.

I have vendored / modified an implementation of that function from third-party utility set remix-utils. (Using as a dependency is not possible because of incompatible TS syntax.)

It's still the best effort and may also need improvements over header key prioritization.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 22, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.5 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 60.29 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.16 KB (-0.02% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 53.63 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 19.9 KB (0%)
@sentry/browser - Webpack (minified) 65.12 KB (0%)
@sentry/react - Webpack (gzipped + minified) 19.92 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 45.93 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.37 KB (0%)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.78 KB (+0.01% 🔺)

@onurtemizkan onurtemizkan linked an issue Nov 23, 2022 that may be closed by this pull request
3 tasks
Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

Looks good!

packages/remix/src/utils/getIpAddress.ts Outdated Show resolved Hide resolved
Co-authored-by: Katie Byers <katie.byers@sentry.io>
@AbhiPrasad AbhiPrasad enabled auto-merge (squash) November 24, 2022 10:20
@AbhiPrasad AbhiPrasad merged commit b52336b into master Nov 24, 2022
@AbhiPrasad AbhiPrasad deleted the onur/remix-extract-user-id branch November 24, 2022 12:32
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.

Remix - No URL included with issues
3 participants