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.

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

Jump to bottom

fix(core): rewrite asset protocol streaming, closes #6375 #6390

Merged
merged 18 commits into from
May 23, 2023
Merged

fix(core): rewrite asset protocol streaming, closes #6375 #6390

merged 18 commits into from
May 23, 2023

Conversation

amrbashir
Copy link
Member

@amrbashir amrbashir commented Mar 2, 2023 •
edited
Loading

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@amrbashir amrbashir requested a review from a team as a code owner March 2, 2023 19:05
@amrbashir amrbashir linked an issue Mar 2, 2023 that may be closed by this pull request
[bug] Asset protocol crashes app with large video files #6375
Closed
@amrbashir amrbashir mentioned this pull request Mar 2, 2023
Closed
@ZerNico
Copy link

ZerNico commented Apr 10, 2023

The rewrite fails to get the mime type on files where the file size is below 8192 bytes.

@amrbashir
Copy link
Member Author

amrbashir commented Apr 11, 2023 •
edited
Loading

@ZerNico Does it panic and fail to load the file or default to text/html? Could you provide a minimal repro?

@amrbashir amrbashir force-pushed the fix/asset-protocol-streaming branch from 5e17cf9 to ca4e238 Compare April 11, 2023 01:41
@ZerNico
Copy link

ZerNico commented Apr 11, 2023

@amrbashir It errors with [Error] Origin http://localhost:1420 is not allowed by Access-Control-Allow-Origin. Status code: 404 in the browser. I am pretty sure it's from the code to get the mime_type because of let mut magic_bytes = [0; 8192];. It tries to read the file to 8192 bytes but fails as the file is smaller than that.

Here is a minimal reproduction. The 2 txt files to test are in the root of the project.
https://github.com/ZerNico/tauri-asset-bug/

@amrbashir amrbashir force-pushed the fix/asset-protocol-streaming branch from bbcf31a to 1293400 Compare April 13, 2023 17:23
@amrbashir
Copy link
Member Author

@ZerNico thanks for testing it out, it should be fixed now, please give it another test.

@ZerNico
Copy link

ZerNico commented Apr 14, 2023

@amrbashir awesome, works fine now.

@lucasfernog lucasfernog requested a review from a team May 23, 2023 14:41
@lucasfernog lucasfernog mentioned this pull request May 23, 2023
Closed
13 tasks
@lucasfernog lucasfernog merged commit 45330e3 into dev May 23, 2023
@lucasfernog lucasfernog deleted the fix/asset-protocol-streaming branch May 23, 2023 18:30
@wusyong wusyong added the security: needs audit This issue/PR needs a security audit label May 25, 2023
lucasfernog added a commit that referenced this pull request May 30, 2023
@lucasfernog @wusyong
@FabianLars @simonhyll @dependabot @renovate @amrbashir @github-actions @chippers @Raphiiko @roniemartinez @hanaTsuk1 @nathan-fall @oppiliappan @kurikomoe @goenning @biaocy @mrjackwills @Mehrbod2002 @Risto-Stevcev @soumt-r @yutotnh @agmmnn @NathanaelRea @shuma @snorkysnark @Lej77 @TDiblik @JonasKruckenberg @Pascal-So @meowtec @yuezk
Merge remote-tracking branch 'origin/dev' into next (#7067)
a50f24b 
Co-authored-by: wusyong <[email protected]>
Co-authored-by: Fabian-Lars <[email protected]>
Co-authored-by: Lucas Nogueira <[email protected]>
Co-authored-by: Simon Hyll <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Co-authored-by: Lucas Nogueira <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Co-authored-by: Amr Bashir <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: chip <[email protected]>
Co-authored-by: Raphii <[email protected]>
Co-authored-by: Ronie Martinez <[email protected]>
Co-authored-by: hanaTsuk1 <[email protected]>
Co-authored-by: nathan-fall <[email protected]>
Co-authored-by: Akshay <[email protected]>
Co-authored-by: KurikoMoe <[email protected]>
Co-authored-by: Guilherme Oenning <[email protected]>
Co-authored-by: Pierre Cashon <[email protected]>
Co-authored-by: Jack Wills <[email protected]>
Co-authored-by: Amirhossein Akhlaghpour <[email protected]>
Co-authored-by: Risto Stevcev <[email protected]>
Co-authored-by: Soumt <[email protected]>
Co-authored-by: yutotnh <[email protected]>
Co-authored-by: Gökçe Merdun <[email protected]>
Co-authored-by: Nathanael Rea <[email protected]>
Co-authored-by: Usman Rajab <[email protected]>
Co-authored-by: Francis The Basilisk <[email protected]>
Co-authored-by: Lej77 <[email protected]>
Co-authored-by: Tomáš Diblík <[email protected]>
Co-authored-by: Jonas Kruckenberg <[email protected]>
Co-authored-by: Pascal Sommer <[email protected]>
Co-authored-by: Bo <[email protected]>
Co-authored-by: Kevin Yue <[email protected]>
fixed grammar and typos (#6937)
Fix api.js docs pipeline with updated typedoc dependencies (#6945)
closes #6887 (#6922)
fix(core): Fix `WindowBuilder::on_navigation` handler never registerd, closes #6865 (#6921)
fix(core): Fix `WindowBuilder::on_navigation` handler never registerd, closes #6865
fix broken symlinks in license files (#6336)
fix(cli): fix cli connection timeout to dev server (fix #6045) (#6046)
fix(bundler): ensure that there are no duplicate extension arguments when bundling on Windows, fixes #6103 (#6917)
fix(bundler): ensure that there are no duplicate extension arguments during bundling on Windows (fix #6103)
closes #5491 (#6408)
fix(nsis): prefill $INSTDIR with previous install path and respect `/D` flag, closes #6928 (#6935)
fix(nsis): prefill $INSTDIR with previous install path and respect `/D` flag, closes #6928
fix(updater): emit `UPTODATE` when server responds with 204, closes #6934 (#6970)
fix(core): unpin all dependencies, closes #6944 (#6966)
fix(bundler): Add new lang_file option in persian variant. (#6972)
fix(core/ipc): access url through webview native object, closes #6889 (#6976)
fix(core): remove trailing slash in http scope url, closes #5208 (#6974)
fix(core): remove trailing slash in http scope url, closes #5208
fix(cli): find correct binary when `--profile` is used, closes #6954 (#6979)
fix(cli): find correct binary when `--profile` is used, closes #6954
closes #6955 (#6987)
closes #6955
closes #6158 (#6969)
closes #6158
fix(cli): improve vs build tools detection (#6982)
fix: updated appimage script to follow symlinks for /usr/lib* (fix: #6992) (#6996)
fix(cli): correctly remove Cargo features (#7013)
Fix typo (#7012)
fix(cli): revert metadata.json field rename from #6795 (#7029)
closes #6732 (#6736)
fix: add missing file properties on Windows, closes #6676 (#6693)
fix(cli.js): detect node-20 binary (#6667)
fix version-or-publish workflow (#7031)
fix(cli/devserver): inject autoreload into HTML only, closes #6997 (#7032)
fix(bundler/nsis): write installer templates UTF16LE encoded, closes #7036 (#7040)
fix(bundler/nsis): write installer templates UTF16LE encoded, closes #7036
fix(core): rewrite `asset` protocol streaming, closes #6375 (#6390)
closes #5939 (#5960)
fix(core): use `safe_block_on` (#7047)
closes #6859 (#6933)
closes #6955 (#6998)
fix(core): populate webview_attrs from config, closes #6794 (#6797)
closes #5176 (#5180)
fix: sound for notifications on windows (fix #6652) (#6680)
close native window's buttons, closes #2353 (#6665)
fix(bundler/nsis): calculate accurate app size, closes #7056 (#7057)
fix(tests): only download update when it is available (#7061)
closes #6706 (#6712)
fix(doc): correct the doc of `content_protected()` (#7065)
closes #6472 (#6530)
fix(macros): use full path to Result to avoid issues with type aliases (#7071)
@FabianLars FabianLars mentioned this pull request Jun 13, 2023
Closed
@chippers chippers added security: reviewed This issue/PR has been review by wg-security and removed security: needs audit This issue/PR needs a security audit labels Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasfernog lucasfernog lucasfernog approved these changes

Assignees
No one assigned
Labels
security: reviewed This issue/PR has been review by wg-security
Projects
Roadmap
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug] Asset protocol crashes app with large video files
5 participants
@amrbashir @ZerNico @lucasfernog @chippers @wusyong