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(http): remove unexpectedly changing origin header when unsafe-headers feature enabled #1703

Closed

Conversation

PeraSite
Copy link

When you use the unsafe-headers feature flag, the website displays CORS-related errors, because when you turn on the unsafe-headers feature flag, tauri automatically set the Origin header to localhost.
This is very confusing, and it is impossible to debug unless you look at the source code.
It will have to be up to developer to set up Origin header value not tauri.
And it doesn't even fit the name "unsafe-headers".

In the first place, why tauri filter the headers by the unsafe-headers feature?
When I first used plugin-http, I had a hard time because a specific header was not recognized by tauri.
May I know why this feature was introduced?
If it's a security issue, it should be fully explained in documentation.

@PeraSite PeraSite requested a review from a team as a code owner August 29, 2024 06:18
@FabianLars FabianLars requested a review from amrbashir September 2, 2024 08:29
@amrbashir
Copy link
Member

The origin is automatically set, as requested in #1167, you can override this behavior by setting the origin using unsafe-headers feature flag or we can add a new option to disable this behavior.

@PeraSite
Copy link
Author

PeraSite commented Sep 2, 2024

I think it's very confusing to make "automatically setting Origin header" behaviour be able to turn off by turning on another feature flag.
It's just different between possible to change Origin headers and forces default Origin value to some other value.

For convenience, I wish that code gets removed.

@amrbashir
Copy link
Member

I don't mind having an option to disable setting the origin header, and doesn't need to be a feature flag, it could be an option set on each fetch request

Copy link
Contributor

github-actions bot commented Sep 3, 2024

Package Changes Through 0e1fcb4

There are 57 changes which include dialog with prerelease, dialog-js with prerelease, fs with prerelease, global-shortcut with prerelease, single-instance with prerelease, store with prerelease, authenticator with prerelease, autostart with prerelease, barcode-scanner with prerelease, biometric with prerelease, cli with prerelease, clipboard-manager with prerelease, deep-link with prerelease, http with prerelease, localhost with prerelease, log-plugin with prerelease, nfc with prerelease, notification with prerelease, os with prerelease, persisted-scope with prerelease, positioner with prerelease, process with prerelease, shell with prerelease, sql with prerelease, stronghold with prerelease, updater with prerelease, upload with prerelease, websocket with prerelease, window-state with prerelease, authenticator-js with prerelease, autostart-js with prerelease, barcode-scanner-js with prerelease, biometric-js with prerelease, cli-js with prerelease, clipboard-manager-js with prerelease, deep-link-js with prerelease, fs-js with prerelease, global-shortcut-js with prerelease, http-js with prerelease, log-js with prerelease, nfc-js with prerelease, notification-js with prerelease, os-js with prerelease, positioner-js with prerelease, process-js with prerelease, shell-js with prerelease, sql-js with prerelease, store-js with prerelease, stronghold-js with prerelease, updater-js with prerelease, upload-js with prerelease, websocket-js with prerelease, window-state-js with prerelease, haptics with prerelease, haptics-js with prerelease, geolocation with prerelease, geolocation-js with prerelease

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
api-example 2.0.0-rc.1 2.0.0-rc.2
api-example-js 2.0.0-rc.1 2.0.0-rc.2
deep-link-example-js 2.0.0-beta.11 2.0.0-rc.0
authenticator 2.0.0-rc.0 2.0.0-rc.1
authenticator-js 2.0.0-rc.0 2.0.0-rc.1
autostart 2.0.0-rc.0 2.0.0-rc.1
autostart-js 2.0.0-rc.0 2.0.0-rc.1
barcode-scanner 2.0.0-rc.2 2.0.0-rc.3
barcode-scanner-js 2.0.0-rc.0 2.0.0-rc.1
biometric 2.0.0-rc.2 2.0.0-rc.3
biometric-js 2.0.0-rc.0 2.0.0-rc.1
cli 2.0.0-rc.0 2.0.0-rc.1
cli-js 2.0.0-rc.0 2.0.0-rc.1
clipboard-manager 2.0.0-rc.2 2.0.0-rc.3
clipboard-manager-js 2.0.0-rc.0 2.0.0-rc.1
deep-link 2.0.0-rc.1 2.0.0-rc.2
deep-link-js 2.0.0-rc.0 2.0.0-rc.1
fs 2.0.0-rc.0 2.0.0-rc.1
fs-js 2.0.0-rc.1 2.0.0-rc.2
dialog 2.0.0-rc.2 2.0.0-rc.3
dialog-js 2.0.0-rc.0 2.0.0-rc.1
geolocation 2.0.0-rc.2 2.0.0-rc.3
geolocation-js 2.0.0-rc.0 2.0.0-rc.1
global-shortcut 2.0.0-rc.1 2.0.0-rc.2
global-shortcut-js 2.0.0-rc.0 2.0.0-rc.1
haptics 2.0.0-rc.2 2.0.0-rc.3
haptics-js 2.0.0-rc.0 2.0.0-rc.1
http 2.0.0-rc.0 2.0.0-rc.1
http-js 2.0.0-rc.1 2.0.0-rc.2
localhost 2.0.0-rc.0 2.0.0-rc.1
log-plugin 2.0.0-rc.1 2.0.0-rc.2
log-js 2.0.0-rc.0 2.0.0-rc.1
nfc 2.0.0-rc.2 2.0.0-rc.3
nfc-js 2.0.0-rc.0 2.0.0-rc.1
notification 2.0.0-rc.2 2.0.0-rc.3
notification-js 2.0.0-rc.0 2.0.0-rc.1
os 2.0.0-rc.0 2.0.0-rc.1
os-js 2.0.0-rc.0 2.0.0-rc.1
persisted-scope 2.0.0-rc.0 2.0.0-rc.1
positioner 2.0.0-rc.0 2.0.0-rc.1
positioner-js 2.0.0-rc.0 2.0.0-rc.1
process 2.0.0-rc.0 2.0.0-rc.1
process-js 2.0.0-rc.0 2.0.0-rc.1
shell 2.0.0-rc.2 2.0.0-rc.3
shell-js 2.0.0-rc.0 2.0.0-rc.1
single-instance 2.0.0-rc.0 2.0.0-rc.1
sql 2.0.0-rc.0 2.0.0-rc.1
sql-js 2.0.0-rc.0 2.0.0-rc.1
store 2.0.0-rc.2 2.0.0-rc.3
store-js 2.0.0-rc.0 2.0.0-rc.1
stronghold 2.0.0-rc.0 2.0.0-rc.1
stronghold-js 2.0.0-rc.0 2.0.0-rc.1
updater 2.0.0-rc.1 2.0.0-rc.2
updater-js 2.0.0-rc.0 2.0.0-rc.1
upload 2.0.0-rc.0 2.0.0-rc.1
upload-js 2.0.0-rc.0 2.0.0-rc.1
websocket 2.0.0-rc.0 2.0.0-rc.1
websocket-js 2.0.0-rc.0 2.0.0-rc.1
window-state 2.0.0-rc.1 2.0.0-rc.2
window-state-js 2.0.0-rc.0 2.0.0-rc.1

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@PeraSite
Copy link
Author

PeraSite commented Sep 7, 2024

yeah adding option to each fetch call would be better because dev doesn't need to read external document for that.

@amrbashir
Copy link
Member

amrbashir commented Sep 10, 2024

@PeraSite would you like to implement the option in this PR or would you like to close this and create an issue instead of me or someone else can work on it?

@PeraSite
Copy link
Author

@PeraSite would you like to implement the option in this PR or would you like to close this and create an issue instead of me or someone else can work on it?

Let me close this PR first.
And after that, I'll post a new PR to edit ClientOptions type to have unsafe-header boolean.
Also remove #[cfg(not(feature = "unsafe-headers"))] and is_unsafe_header part from the Rust codebases.
Adding fetch parameter should give better experience than Rust's feaature flag.

@PeraSite PeraSite closed this Sep 11, 2024
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.

2 participants