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

Add option to passthrough headers inside check() #1634

Closed
pacosw1 opened this issue Aug 7, 2024 · 7 comments · Fixed by #1661
Closed

Add option to passthrough headers inside check() #1634

pacosw1 opened this issue Aug 7, 2024 · 7 comments · Fixed by #1661
Labels
enhancement New feature or request plugin: updater

Comments

@pacosw1
Copy link

pacosw1 commented Aug 7, 2024

I am currently implementing a dynamic server updater using the rc plugin. However, I can't have the update bundles hosted publicly so I implemented a system to store them in S3.

To cut to the chase, I create a presigned url and this is what is returned from my update endpoint for the app to download, however, since I add authentication headers, the plugin adds them to the request and it caused a 400 bad request from S3 side as the signature doesn't match and in my case, "Authorization" header can't be used since presigned url uses a different auth mechanism.

I worked around this by using the rust implementation to remove the headers from the update object, but this isn't possible in javascript.

It would be nice to have an option {"passthroughHeaders": bool} set to true by default, or some other way to control the headers when downloading the update.

@pacosw1
Copy link
Author

pacosw1 commented Aug 7, 2024

I'm not sure how to add the plugin tag

@FabianLars FabianLars added enhancement New feature or request plugin: updater labels Aug 8, 2024
@amrbashir
Copy link
Member

Could you explain a bit more what would passthroughHeaders would do? do you want to remove some headers when downloading or add new headers?

@pacosw1
Copy link
Author

pacosw1 commented Aug 13, 2024

I think passing the headers by default is a good idea and will work with most use cases (github action, releases etc...), but I am using an S3 presigned url.

Because it is signed, I can't have tauri add http headers because it breaks the signature. In my case, and probably most custom solutions for the updater component, it's best to give an option to modify the headers, or disable passing the headers to the "download" function

Currently in the js client, we have no control over the http request.

@amrbashir
Copy link
Member

I see, we can add an optional second argument to allow modifying some options.

@amrbashir
Copy link
Member

amrbashir commented Aug 14, 2024

@pacosw1 can you see if #1634 fixes this for you? you can clear the headers like this:

update.download(() => {}, { headers: [] })

On a side note, there is two headers we set internally, Accept and User-Agent:

  • User-Agent is the same value when checking for an update and when downloading so I don't suspect it is the problem.
  • Accept has different value, when checking for update, it is set to application/json and when downloading, it is set to application/octet-stream but seeing that you used the Rust APIs and it worked for you, I am assuming this is not an issue as well.

Let me know if these internal headers is causing a problem

@pacosw1
Copy link
Author

pacosw1 commented Aug 14, 2024

@pacosw1 can you see if #1634 fixes this for you? you can clear the headers like this:

update.download(() => {}, { headers: [] })

On a side note, there is two headers we set internally, Accept and User-Agent:

  • User-Agent is the same value when checking for an update and when downloading so I don't suspect it is the problem.

  • Accept has different value, when checking for update, it is set to application/json and when downloading, it is set to application/octet-stream but seeing that you used the Rust APIs and it worked for you, I am assuming this is not an issue as well.

Let me know if these internal headers is causing a problem

Hey, I will implement this and report back.

Not sure about the other headers, but yeah, I cleared the headers in rust and it worked correctly.

In that case it was probably my "Authorization" header causing the problem.

Thanks for the fix!

@amrbashir
Copy link
Member

amrbashir commented Aug 14, 2024

Not sure about the other headers, but yeah, I cleared the headers in rust and it worked correctly.

even if you clear the headers before calling Update::download on the Rust side, we still add them right before actually sending the HTTP request

Hey, I will implement this and report back.

Thanks, please let me know how it goes

Sir-Thom pushed a commit to Sir-Thom/plugins-workspace that referenced this issue Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin: updater
Projects
None yet
3 participants