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

SplitHTTP: Fix wrong config deserialization #3610

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Jul 29, 2024

Manual testing was conducted only using explicitly set parameters. However, when the parameters are not explicitly set in JSON config, it seems that c.MaxUploadSize will contain RandRangeConfig { From: 0, To: 0 } instead of nil, which breaks upload entirely.

I now tested all combinations and can confirm that h1, h2, h3 work fine against both new and old servers.

Testing was conducted only using explicit parameters, and using
testsuite. However, when the parameters are not explicitly set in JSON
config, it seems that `c.MaxUploadSize` will contain `RandRangeConfig {
From: 0, To: 0 }` instead of `nil`, which breaks upload entirely.
@RPRX RPRX merged commit 33daa0c into XTLS:main Jul 29, 2024
36 checks passed
@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

想把 maxUploadSize 改名为 maxUploadBytes,把服务端默认值也设为 1MB,把服务端 maxConcurrentUploads 默认值也设为 100

改成 maxEachUploadBytes

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

改成 maxEachUploadBytes

@Fangliding
Copy link
Member

Maybe like this?

MaxConcurrentUploads *Int32Range        `json:"maxConcurrentUploads"`
if c.MaxConcurrentUploads != nil {
...
}

@mmmray
Copy link
Collaborator Author

mmmray commented Jul 29, 2024

@Fangliding you're right, this seems to just be a case of wrong types in the json struct... since this works for now, i will fix it properly later, once the dust settles. thanks

@RPRX
Copy link
Member

RPRX commented Jul 29, 2024

@Fangliding you're right, this seems to just be a case of wrong types in the json struct... since this works for now, i will fix it properly later, once the dust settles. thanks

没问题的话我就先发 v1.8.23 了,整理代码的工作可以放到后面,我还想把那三个字段的顺序改一下

@mmmray
Copy link
Collaborator Author

mmmray commented Jul 29, 2024

1.8.23 can be released, nothing can go wrong.

the order.. ok

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.

3 participants