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

bug: aegir/fixtures loadFixtures incorrectly alters bytes in the browser #1462

Closed
SgtPooki opened this issue Feb 7, 2024 · 6 comments
Closed

Comments

@SgtPooki
Copy link
Member

SgtPooki commented Feb 7, 2024

Describe the bug

When using the loadFixtures function from aegir/fixtures in the browser, the bytes returned are incorrectly altered, causing test failures in https://github.com/SgtPooki/file-type.

Repro code in branch at https://github.com/SgtPooki/file-type/pull/new/repro/aegir-load-fixtures-browser-bug

To Reproduce
Steps to reproduce the behavior:

  1. git clone https://github.com/SgtPooki/file-type
  2. cd file-type
  3. git checkout repro/aegir-load-fixtures-browser-bug
  4. npm i && npm run test

Expected behavior
The browser tests pass, the same as they do in the node tests.

Screenshots

  1) fixture.tar.gz 65 .fileTypeFromBuffer() method - same fileType
  65 passing (605ms)
  1 failing
  1) fixture.tar.gz 65 .fileTypeFromBuffer() method - same fileType:

      AssertionError: expected 'tar' to equal 'gz'
      + expected - actual

      -tar
      +gz
      
      at checkBufferLike (/Users/sgtpooki/code/work/foss/SgtPooki/file-type/test/file-type.spec.ts:262:18)
      at testFromBuffer (/Users/sgtpooki/code/work/foss/SgtPooki/file-type/test/file-type.spec.ts:287:3)
      at Context2.<anonymous> (/Users/sgtpooki/code/work/foss/SgtPooki/file-type/test/file-type.spec.ts:349:9)

Desktop (please complete the following information):

  • OS: macOS
  • Browser: N/A
  • Version: aegir@42.2.3

Additional context

The specific failing test can be ran by running npm run test -- -g 'fixture.tar.gz 65'. You'll see that the test passes in node, but fails in the browser test.

╰─ ✔ ❯ npm run test -- -g 'fixture.tar.gz 65'

> @sgtpooki/file-type@1.0.0 test
> aegir test -g fixture.tar.gz 65

build

> @sgtpooki/file-type@1.0.0 build
> npm run dep-check && aegir build


> @sgtpooki/file-type@1.0.0 dep-check
> aegir dep-check

  ✔ dependency-check
[12:12:52] tsc [started]
[12:12:52] tsc [completed]
[12:12:52] esbuild [started]
[12:12:52] esbuild [completed]
test node.js
Warning: Cannot find any files matching pattern "test/node.{js,cjs,mjs}"
Warning: Cannot find any files matching pattern "test/**/*.spec.{js,cjs,mjs}"


  ✔ fixture.tar.gz 65 .fileTypeFromBuffer() method - same fileType

  1 passing (12ms)

test browser
ℹ Browser "chromium" setup complete.

  1) fixture.tar.gz 65 .fileTypeFromBuffer() method - same fileType
  0 passing (147ms)
  1 failing
  1) fixture.tar.gz 65 .fileTypeFromBuffer() method - same fileType:

      AssertionError: expected 'tar' to equal 'gz'
      + expected - actual

      -tar
      +gz

      at checkBufferLike (/Users/sgtpooki/code/testing/SgtPooki/file-type/test/file-type.spec.ts:262:18)
      at testFromBuffer (/Users/sgtpooki/code/testing/SgtPooki/file-type/test/file-type.spec.ts:287:3)
      at Context2.<anonymous> (/Users/sgtpooki/code/testing/SgtPooki/file-type/test/file-type.spec.ts:349:9)

✘ Tests failed.
Command failed with exit code 1: /Users/sgtpooki/code/testing/SgtPooki/file-type/node_modules/.bin/pw-test test/**/*.spec.{js,cjs,mjs} test/browser.{js,cjs,mjs} dist/test/**/*.spec.{js,cjs,mjs} dist/test/browser.{js,cjs,mjs} --mode main --runner mocha --config /Users/sgtpooki/code/testing/SgtPooki/file-type/node_modules/aegir/src/config/pw-test.js --timeout=60000 --grep=fixture.tar.gz 65 --bail
@achingbrain
Copy link
Member

achingbrain commented Feb 8, 2024

I think the problem here is that your test loads a gzipped tarball, and something is decompressing it before returning it as the responseText property of the XMLHttpRequest object.

The bit-twiddling comes from here btw - unfortunately the bytes have already been decompressed by this point so the bug is not there.

@achingbrain
Copy link
Member

I think the bug is in sirv, used by playwright-test to serve fixtures.

lukeed/sirv#158

A workaround is to just rename the .tar.gz file so it doesn't end in .gz.

achingbrain added a commit to hugomrdias/playwright-test that referenced this issue Feb 8, 2024
When loading a file that ends in `.gz`, `sirv` will set the
`Content-Encoding` header to `gzip` which means browsers will
unzip the content before handing it back to `fetch` or `XMLHttpRequest`.

This PR adds a workaround to the asset server that sets the header
to a garbage value if a file ending in `.gz` has been requested.

It's necessary to use a garbage value because `sirv` will only set
the header if it's not been set already, so we can't simply delete it.

Refs: lukeed/sirv#158
Refs: ipfs/aegir#1462
hugomrdias pushed a commit to hugomrdias/playwright-test that referenced this issue Feb 8, 2024
When loading a file that ends in `.gz`, `sirv` will set the
`Content-Encoding` header to `gzip` which means browsers will
unzip the content before handing it back to `fetch` or `XMLHttpRequest`.

This PR adds a workaround to the asset server that sets the header
to a garbage value if a file ending in `.gz` has been requested.

It's necessary to use a garbage value because `sirv` will only set
the header if it's not been set already, so we can't simply delete it.

Refs: lukeed/sirv#158
Refs: ipfs/aegir#1462
@SgtPooki
Copy link
Member Author

SgtPooki commented Feb 8, 2024

A workaround is to just rename the .tar.gz file so it doesn't end in .gz.

But the gz is the expected type, which returns correctly with a simple http server and piping fs.createReadStream

@SgtPooki
Copy link
Member Author

SgtPooki commented Feb 8, 2024

Thanks for the quick responses and bugfix in playright-test

@achingbrain
Copy link
Member

achingbrain commented Feb 9, 2024

But the gz is the expected type, which returns correctly with a simple http server and piping fs.createReadStream

Renaming the file doesn't make it a different type, it just stops sirv setting the Content-Encoding header that causes fetch/XMLHttpRequest to decompress the bytes before making them available.

It's just a workaround to unblock you while the issue is fixed upstream.

achingbrain added a commit that referenced this issue Feb 12, 2024
Test loading gzipped fixtures, particularly in browsers.

Prevents regressions of #1462
achingbrain added a commit that referenced this issue Feb 12, 2024
Test loading gzipped fixtures, particularly in browsers.

Prevents regressions of #1462
@achingbrain
Copy link
Member

Closing as a workaround has been released in hugomrdias/playwright-test#644 so this is usable now and the proper fix is discussed in lukeed/sirv#158

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants