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.

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

Conversation

Legend-Master
Copy link
Contributor

@Legend-Master Legend-Master commented Apr 14, 2025

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

@github-project-automation github-project-automation bot moved this to 📬Proposal in Roadmap Apr 14, 2025
Copy link
Contributor

github-actions bot commented Apr 14, 2025

Package Changes Through 8448fd2

There 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 Versions

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

package current next
@tauri-apps/api 2.4.1 2.5.0
tauri-utils 2.3.1 2.4.0
tauri-bundler 2.3.1 2.4.0
tauri-runtime 2.5.1 2.6.0
tauri-runtime-wry 2.5.1 2.6.0
tauri-codegen 2.1.1 2.1.2
tauri-macros 2.1.1 2.1.2
tauri-plugin 2.1.1 2.1.2
tauri-build 2.1.1 2.1.2
tauri 2.4.1 2.5.0
@tauri-apps/cli 2.4.1 2.5.0
tauri-cli 2.4.1 2.5.0

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


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

@WSH032
Copy link
Contributor

WSH032 commented Apr 14, 2025

Thanks! I can confirm it fixed the issue.

@Legend-Master Legend-Master marked this pull request as ready for review April 14, 2025 05:28
@Legend-Master Legend-Master requested a review from a team as a code owner April 14, 2025 05:28
Comment on lines 30 to 44
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

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 :)

Seems like we have changed it in tauri-apps#9530 deliberately,
so preserving it in this change
lucasfernog
lucasfernog previously approved these changes Apr 14, 2025
Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

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

awesome

@lucasfernog lucasfernog merged commit f888502 into tauri-apps:dev Apr 14, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from 📬Proposal to 🔎 In audit in Roadmap Apr 14, 2025
@Legend-Master Legend-Master deleted the use-headers-in-send-ipc-message branch April 15, 2025 01:25
WSH032 added a commit to pytauri/pytauri that referenced this pull request Apr 20, 2025
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`
WSH032 added a commit to pytauri/pytauri that referenced this pull request Apr 21, 2025
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`
github-merge-queue bot pushed a commit to pytauri/pytauri that referenced this pull request Apr 21, 2025
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔎 In audit
Development

Successfully merging this pull request may close these issues.

[bug] invoke does not handle Headers correctly
3 participants