Skip to content

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 14, 2025
Merged

fix(core): use Headers in sendIpcMessage #13227

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c854a0d
Use `Headers` in `sendIpcMessage`
Legend-Master Apr 14, 2025
857e50f
Add change file
Legend-Master Apr 14, 2025
3913be8
Change files
Legend-Master Apr 14, 2025
70ebd74
Don't use optional chaining
Legend-Master Apr 14, 2025
cc38360
do not let the tauri headers to be overwritten
lucasfernog Apr 14, 2025
8448fd2
use HeadersInit on the type definition
lucasfernog Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/invoke-headers-non-ascii.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tauri": patch:bug
---

`invoke` will now properly throw when `options.headers` contains non-ascii characters instead of silently replacing them
5 changes: 5 additions & 0 deletions .changes/invoke-headers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tauri": patch:bug
---

Fix `invoke` ignores the headers option if it's an `Headers`
23 changes: 16 additions & 7 deletions crates/tauri/scripts/ipc-protocol.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,25 @@
&& (canUseCustomProtocol || cmd === fetchChannelDataCommand)
) {
const { contentType, data } = processIpcMessage(payload)
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)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Contributor Author

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

Copy link
Member

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)

Copy link
Member

@lucasfernog lucasfernog Apr 14, 2025 •
edited
Loading

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed :)

fetch(window.__TAURI_INTERNALS__.convertFileSrc(cmd, 'ipc'), {
method: 'POST',
body: data,
headers: {
'Content-Type': contentType,
'Tauri-Callback': callback,
'Tauri-Error': error,
'Tauri-Invoke-Key': __TAURI_INVOKE_KEY__,
...((options && options.headers) || {})
}
headers
})
.then((response) => {
const cb =
Expand Down
Loading