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

Fixed bug: tus failed to upload chunk at offset 0 over https #4154

Merged
merged 6 commits into from
Jan 13, 2022

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Jan 12, 2022

Motivation and context

Currently if we use tus upload for cvat tasks over https error like this is displayed:
1
That happens because of incorrect protocol in chunk upload location returned by server. Client makes https request that is forwarded by traefik to django via http. Thats why django's request.build_absolute_uri returns location with http protocol.

There are several ways to let django know that client request was made over https:

I've used the last option. Based on SECURE_PROXY_SSL_HEADER setting, if X-Forwarded-Proto is correct, then django knows that request was sent via https.

How has this been tested?

Deployed locally and set secure header to http, X-Forwarded-Proto was http, then django knew that connection was secure and returned https protocol location.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@klakhov
Copy link
Contributor Author

klakhov commented Jan 12, 2022

@azhavoro Could you look on these settings please? Is it correct that we don't need to change any traefik settings to keep X-Forwarded-* headers visible over https as we do with http?

CHANGELOG.md Outdated Show resolved Hide resolved
@azhavoro azhavoro self-requested a review January 13, 2022 11:20
cvat/settings/base.py Outdated Show resolved Hide resolved
@nmanovic nmanovic merged commit 57e5e62 into develop Jan 13, 2022
@nmanovic nmanovic deleted the kl/fix-tus-https branch January 13, 2022 14:35
@nmanovic nmanovic mentioned this pull request Mar 4, 2022
7 tasks
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.

4 participants