Fixed issue with cvat running in HTTP mode behind a HTTPS reverse proxy #7313
Conversation
- Check if HTTP_ORIGIN exists in the request.META, if it does, use the same http scheme for the TUS location uri. If not exist, use Django's build_absolute_uri() for the TUS location uri.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes involve modifying the logic for constructing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cvat/apps/engine/mixins.py (1 hunks)
Additional comments not posted (2)
cvat/apps/engine/mixins.py (2)
303-303: LGTM! The change aligns with the PR objective.The modification to prioritize the
HTTP_ORIGINheader is consistent with the goal of handling HTTPS reverse proxy scenarios.
303-303: Verify the correctness oflocationconstruction logic.Ensure that the
HTTP_ORIGINheader is correctly set and used in all relevant scenarios, especially when dealing with cross-origin requests.Run the following script to verify the usage of
HTTP_ORIGINin the codebase:Verification successful
Verified: Correct usage of
HTTP_ORIGINfor location construction.The
HTTP_ORIGINheader is correctly used to construct thelocationvariable incvat/apps/engine/mixins.py. This change is consistent with handling cross-origin requests effectively.
- File:
cvat/apps/engine/mixins.py
- Lines: Usage of
HTTP_ORIGINto constructlocation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `HTTP_ORIGIN` in the codebase. # Test: Search for occurrences of `HTTP_ORIGIN`. Expect: Correct usage in relevant scenarios. rg --type python 'HTTP_ORIGIN'Length of output: 61
Script:
#!/bin/bash # Description: Verify the usage of `HTTP_ORIGIN` in the codebase. # Test: Search for occurrences of `HTTP_ORIGIN`. Expect: Correct usage in relevant scenarios. rg 'HTTP_ORIGIN'Length of output: 207
|
|
any chance this will get merged soon ? |
|
Hi cargoculttech Does the following look right? Thanks |
|
Hi cargoculttech |
Issue: #7288
Motivation and context
Fixed the problem where TUS resumable upload feature does not work when CVAT is setup to run in HTTP only, but put behind a HTTPs proxy.
Scenario: Client --> HTTPS reverse proxy --> HTTP Cvat server
Originally, CVAT would return HTTP location for tus resumable, but since client is requesting CVAT through a HTTPS proxy, CVAT should also return a HTTPS location for TUS resumable.
The change allows CVAT to recognize that it's behind a HTTPS reverse proxy, and return the HTTP scheme accordingly. This
change also works if
Client --> HTTP reverse proxy --> HTTPS Cvat server.This change is required because it makes CVAT easier to be hosted behind a reverse proxy.
How has this been tested?
I set up a public nginx proxy manager on https://dev.cvat.mycompany.com, and it proxies to http://192.168.1.123:8080 (my cvat machine). Features such as
import datasetdidn't work because it uses TUS Resumable, which was returning the wrong HTTP scheme. After the change, the feature works when the proxy server and cvat runs in different HTTP scheme.Checklist
If an item isn't applicable for some reason, then
explicitly strikethroughthe wholeline. If you don't do that, GitHub will show incorrect progress for the pull request.
If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
developbranch- [ ] I have created a changelog fragment- [ ] I have updated the documentation accordingly- [ ] I have added tests to cover my changes- [ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
HTTP_ORIGINheader for location construction.Bug Fixes