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

hyper: fix a progress upload counter bug #11780

Closed
wants to merge 1 commit into from

Conversation

nnethercote
Copy link
Contributor

Curl_pgrsSetUploadCounter should be a passed a total count, not an increment.

This changes the failing diff for test 579 with hyper from this:

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]

to this:

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]

Presumably a step in the right direction.

@github-actions github-actions bot added the Hyper label Sep 1, 2023
`Curl_pgrsSetUploadCounter` should be a passed a total count, not an
increment.

This changes the failing diff for test 579 with hyper from this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]
```
to this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]
```
Presumably a step in the right direction.
@nnethercote nnethercote changed the title hyper: fix a progress upload counter bug. hyper: fix a progress upload counter bug Sep 1, 2023
@nnethercote
Copy link
Contributor Author

 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]

I more or less understand the differences here now. The test has this data that gets sent:

static const char * const post[]={
  "one",
  "two",
  "three",
  "and a final longer crap: four",
  NULL
};

Vanilla curl gets an update for each entry, which is why there are five entries after the initial UL 0. The length of each update is the length of the string, plus another 5 or 6 chars for a hexlen and some EOL chars. That's how we end up with (3+5) + (3+5) + (5+5) + (29+6) + (0+5) = 66 bytes.

curl+hyper ends up doing things slightly differently: it doesn't include the hexlen/EOL chars, so we end up with 3 + 3 + 5 + 29 + 0 = 40 chars. Also, it doesn't call Curl_pgrsUpdate between each Curl_pgrsSetUploadCounter, so we get a single update at the end instead of multiple updates.

I don't know if these differences are important. Maybe it's worth having different expected output for the hyper build?

@bagder bagder closed this in 73f4ef5 Sep 1, 2023
@bagder
Copy link
Member

bagder commented Sep 1, 2023

Thanks!

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
`Curl_pgrsSetUploadCounter` should be a passed a total count, not an
increment.

This changes the failing diff for test 579 with hyper from this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 29 out of 0[LF]
```
to this:
```
 Progress callback called with UL 0 out of 0[LF]
-Progress callback called with UL 8 out of 0[LF]
-Progress callback called with UL 16 out of 0[LF]
-Progress callback called with UL 26 out of 0[LF]
-Progress callback called with UL 61 out of 0[LF]
-Progress callback called with UL 66 out of 0[LF]
+Progress callback called with UL 40 out of 0[LF]
```
Presumably a step in the right direction.

Closes curl#11780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants