-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(core): use Headers
in sendIpcMessage
#13227
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(core): use Headers
in sendIpcMessage
#13227
Conversation
Package Changes Through 8448fd2There are 8 changes which include @tauri-apps/api with minor, tauri with minor, tauri-cli with minor, @tauri-apps/cli with minor, tauri-utils with minor, tauri-bundler with minor, tauri-runtime with minor, tauri-runtime-wry with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
Thanks! I can confirm it fixed the issue. |
crates/tauri/scripts/ipc-protocol.js
Outdated
const headers = new Headers({ | ||
'Content-Type': contentType, | ||
'Tauri-Callback': callback, | ||
'Tauri-Error': error, | ||
'Tauri-Invoke-Key': __TAURI_INVOKE_KEY__ | ||
}) | ||
if (options?.headers) { | ||
const overrideHeaders = | ||
options.headers instanceof Headers | ||
? options.headers.entries() | ||
: Object.entries(options.headers) | ||
for (const [key, value] of overrideHeaders) { | ||
headers.set(key, value) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const headers = new Headers({ | |
'Content-Type': contentType, | |
'Tauri-Callback': callback, | |
'Tauri-Error': error, | |
'Tauri-Invoke-Key': __TAURI_INVOKE_KEY__ | |
}) | |
if (options?.headers) { | |
const overrideHeaders = | |
options.headers instanceof Headers | |
? options.headers.entries() | |
: Object.entries(options.headers) | |
for (const [key, value] of overrideHeaders) { | |
headers.set(key, value) | |
} | |
} | |
const headers = new Headers(options?.headers); | |
headers.set("Content-Type", contentType); | |
headers.set("Tauri-Callback", callback); | |
headers.set("Tauri-Error", error); | |
headers.set("Tauri-Invoke-Key", __TAURI_INVOKE_KEY__); |
I'm just a bit curious, why not implement it this way? It seems more elegant.
I guess it's to preserve previous behavior (allowing users to override preset headers)? But overriding these headers seems like an unreasonable behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's to preserve previous behavior (allowing users to override preset headers)?
Yeah, that's the reason why I did it like this
But overriding these headers seems like an unreasonable behavior.
That's true though
I'm not entirely sure if it's supposed to be able to override these or not, cc @lucasfernog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
welllll nice catch.. that's used internally so we shouldn't allow them to be overwritten (it would break the IPC if you did so - never running or responding depending on which value you change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Legend-Master can you take his suggestion and change new Headers(options?.headers)
to new Headers((options && options.headers) || {})
(null cannot be used in that constructor)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed :)
Seems like we have changed it in tauri-apps#9530 deliberately, so preserving it in this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome
Co-authored-by: Sean Wang <[email protected]>
Rust: - tauri: 2.5 - tauri-build: 2.2 - tauri-plugin: 2.2 - tauri-utils: ~2.4 Js: - @tauri-apps/api: ^2.5 BREAKING CHANGE: This requires user to upgrade to `@tauri-apps/api: ^2.5`, corresponding to `tauri-plugin-pytauri-api: 0.5`
Rust: - tauri: 2.5 - tauri-build: 2.2 - tauri-plugin: 2.2 - tauri-utils: ~2.4 Js: - @tauri-apps/api: ^2.5 BREAKING CHANGE: This requires user to upgrade to `@tauri-apps/api: ^2.5`, corresponding to `tauri-plugin-pytauri-api: 0.5`
* feat(pytauri): accessing the request headers in `Commands` Added: - `pytauri.ipc.Headers` * docs: add tip about how to convert `ipc.Headers` to a dictionary * fix!: bump `tauri` to fix tauri-apps/tauri#13227 Rust: - tauri: 2.5 - tauri-build: 2.2 - tauri-plugin: 2.2 - tauri-utils: ~2.4 Js: - @tauri-apps/api: ^2.5 BREAKING CHANGE: This requires user to upgrade to `@tauri-apps/api: ^2.5`, corresponding to `tauri-plugin-pytauri-api: 0.5` * refactor: do not check `pyfunc` header at runtime * fix: bump `tauri` to `2.5.1` to fix tauri-apps/tauri#13268 * docs: changelog
Fix #13223
This also makes it easier to debug cases like tauri-apps/plugins-workspace#1965 where non-ascii characters get passed in to it and it silently replaces them instead of throwing an error
Also, this should make override header keys case insensitive