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

Perform proper URL rewriting when Git Redirector is in use #717

Merged
merged 1 commit into from May 4, 2023

Conversation

oo-bldrs
Copy link
Contributor

@oo-bldrs oo-bldrs commented May 3, 2023

When an IFC load request is being fulfilled, there's a small piece of logic that will rewrite the URL based on if it's a request to GitHub and the Share user is not logged in. Previously, the rewrite target was static: raw.githubusercontent.com but that logic was changed with #716.

In creating that issue (and the subsequent code review), I neglected to clarify that the entire URL should be modified appropriately as opposed to the bad logic of only replacing the host name of the target URL.

This PR fixes that logic to do the correct URL rewriting. This did not affect any users in production because it was never enabled (via setting the environment variable).

@netlify
Copy link

netlify bot commented May 3, 2023

Deploy Preview for bldrs-share ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share/deploys/64529aec0afb8400b48bb8d1
😎 Deploy Preview https://deploy-preview-717--bldrs-share.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pablo-mayrgundter pablo-mayrgundter self-assigned this May 3, 2023
@oo-bldrs oo-bldrs merged commit b65c5d7 into bldrs-ai:main May 4, 2023
7 checks passed
@oo-bldrs oo-bldrs deleted the url-not-hostname branch May 5, 2023 11:23
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.

LFS: Public model opens fine when logged in but not without being logged in
2 participants