Skip to content

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Feb 6, 2024

To check if this closes #2678 you can run

import { fetch } from './index-fetch.js'

const response = await fetch('https://u-vpn.unilim.fr/remote/logincheck', {
  headers: {
    accept: '*/*',
    'cache-control': 'no-store, no-cache, must-revalidate',
    'content-type': 'text/plain;charset=UTF-8',
    'if-modified-since': 'Sat, 1 Jan 2000 00:00:00 GMT',
    pragma: 'no-cache',
    Referer: 'https://u-vpn.unilim.fr/remote/login?lang=en',
    'Referrer-Policy': 'strict-origin-when-cross-origin'
  },
  // Fake credentials, still throws the error though
  body: 'ajax=1&username=a&realm=&credential=b',
  method: 'POST'
})

// Only happens here.
console.log(await response.text())
@mcollina

Can your run some benchmarks on this please?

@ShogunPanda

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (next@cf4d90d). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2705   +/-   ##
=======================================
  Coverage        ?   85.28%           
=======================================
  Files           ?       84           
  Lines           ?     7592           
  Branches        ?        0           
=======================================
  Hits            ?     6475           
  Misses          ?     1117           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 6, 2024

fixed the tests

imho ready for merge.

@Uzlopak Uzlopak changed the title upgrade llhttp chore: upgrade llhttp to 9.1.3 + Feb 6, 2024
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 6, 2024

@mcollina

Maybe we should first release a undici 6.6.2 before we merge this?

@ShogunPanda
Copy link
Contributor

I'm still not sure about the right semver change but yes, let's merge this first.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 6, 2024

@ShogunPanda

I took the version from HEAD of the default branch of llhttp. Can you make a release of llhttp? THan we just fix the version in this PR.

@ShogunPanda
Copy link
Contributor

Sure, I'll release llhttp after lunch.

@ShogunPanda
Copy link
Contributor

@Uzlopak llhttp 9.2.0. has just been released. You can quickly rework this PR and then we're ready to go.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 7, 2024

@ShogunPanda

Thanks. I just patched the version in the header file.

Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@Uzlopak Uzlopak changed the title chore: upgrade llhttp to 9.1.3 + chore: upgrade llhttp to 9.2.0 Feb 7, 2024
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

I think we should add support/expose the leniency flags, and potentially read those values from node.js core itself. That can be done in a follow-up PR.

@Uzlopak Uzlopak changed the base branch from main to next February 9, 2024 11:34
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 9, 2024

@mcollina
@ShogunPanda
@metcoder95
@KhafraDev

Target changed to next branch

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit c2c293f into nodejs:next Feb 9, 2024
@Uzlopak Uzlopak deleted the upgrade-llhttp branch February 9, 2024 18:05
ronag pushed a commit that referenced this pull request Feb 16, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
ronag pushed a commit that referenced this pull request Feb 16, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
ronag added a commit that referenced this pull request Feb 16, 2024
* remove anti-pattern dispatcher hooks (#2723)

* chore: upgrade llhttp to 9.2.0 (#2705)

* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0

---------

Co-authored-by: Aras Abbasi <[email protected]>
ronag pushed a commit that referenced this pull request Feb 16, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
ronag pushed a commit that referenced this pull request Feb 26, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
KhafraDev pushed a commit to KhafraDev/undici that referenced this pull request Jun 24, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
@ronag ronag mentioned this pull request Jul 3, 2024
7 tasks
metcoder95 pushed a commit that referenced this pull request Jul 3, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
metcoder95 pushed a commit that referenced this pull request Jul 3, 2024
* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0
ronag pushed a commit that referenced this pull request Jul 5, 2024
* chore: upgrade llhttp to 9.2.0 (#2705)

* upgrade llhttp

* fix tests

* set version of llhttp 9.2.0

* chore: adjustments

---------

Co-authored-by: Aras Abbasi <[email protected]>
@github-actions github-actions bot mentioned this pull request Dec 3, 2024
This was referenced Dec 16, 2024
@github-actions github-actions bot mentioned this pull request Mar 12, 2025
@github-actions github-actions bot mentioned this pull request May 12, 2025
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

Successfully merging this pull request may close these issues.

HPE_INVALID_CHUNK_SIZE but works without any issue in curl, browser and other runtimes excluding Node.js
6 participants