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

Use the request path base when creating location header urls #210

Merged
merged 6 commits into from
Nov 2, 2023
Merged

Use the request path base when creating location header urls #210

merged 6 commits into from
Nov 2, 2023

Conversation

Louis9902
Copy link
Contributor

Use the request path base when creating url for the upload location header. The base base is only used for the endpoint based invoker, as this is backwards compatible with the previouse behaviour.

The middleware approach always defined the absolut url which is used, and if there are existing configurations which include the path base this will break the backwards compat.

@smatsson
Copy link
Collaborator

Thanks for this PR! This seems like a bug on my end. I'll run some tests and merge if everything looks ok 👍

Are you running tusdotnet as a virtual application in IIS or are you routing it through a subfolder in a reverse proxy?

@Louis9902
Copy link
Contributor Author

It depends, we deploy via IIS or as Standalone with Krestrel and a reverse proxy. When running in IIS it's ran as virtual application.

@smatsson
Copy link
Collaborator

Fixed some issues in the PR (see commits above). Still needs some more testing. Test setup used was a nginx reverse proxy running the following config:

  location /tusdotnet {
            proxy_buffering off;
            proxy_pass http://localhost:5008;
        }

The net6 test app was updated to use this path base:

app.UsePathBase("/tusdotnet/");

@smatsson smatsson merged commit 8ac0ebc into tusdotnet:master Nov 2, 2023
@smatsson smatsson added this to the 2.7.1+1 milestone Nov 2, 2023
smatsson added a commit that referenced this pull request Nov 11, 2023
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.

None yet

2 participants