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

Add --max-filesize option to limit HTTP response #2468

Closed
wants to merge 1 commit into from

Conversation

baerwang
Copy link
Contributor

@baerwang baerwang commented Feb 29, 2024

#2353

develop process

options

image

image

cargo clippy

image

tests_ok/help.sh

image

@baerwang baerwang force-pushed the feat/file-limit branch 2 times, most recently from 010b1d4 to 65ecb0f Compare March 1, 2024 05:46
@baerwang baerwang marked this pull request as ready for review March 1, 2024 05:47
Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

The option max_filesize should also work for [Options]section:

GET https://foo.com
output: bar.bin
max-filesize: 128000
HTTP 200

packages/hurl/src/cli/options/commands.rs Outdated Show resolved Hide resolved
packages/hurl/src/runner/result.rs Outdated Show resolved Hide resolved
packages/hurl/src/runner/result.rs Outdated Show resolved Hide resolved
packages/hurl/src/runner/result.rs Outdated Show resolved Hide resolved
@baerwang baerwang force-pushed the feat/file-limit branch 2 times, most recently from cc06348 to 6585988 Compare March 16, 2024 13:58
Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

Hi @baerwang

To make the CI OK, you should look at the integration test tests_ok/help.sh: this is a simple test that checks the output of hurl --help. When adding a new option, we must update this test.

docs/spec/options/hurl/max_filesize.option Outdated Show resolved Hide resolved
packages/hurl/src/main.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

EnryResult struct doesn't need to change. max_filesize must be passed to run method (with RunnerOptions) so we can configure libcurl to use it. This way we don't have to check the dfile size when we write a response: the exchange will be in error if the file size is greater than the optiions.

@jcamiel
Copy link
Collaborator

jcamiel commented Mar 28, 2024

Ho @baerwang I can help with this PR and push some fixes if you want!

@baerwang
Copy link
Contributor Author

Ho @baerwang I can help with this PR and push some fixes if you want!

i'll changed this code at sunday

Copy link
Collaborator

@jcamiel jcamiel left a comment

Choose a reason for hiding this comment

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

We should use an Option<usize> instead of a usize for max_filesize fields.

@jcamiel
Copy link
Collaborator

jcamiel commented Apr 1, 2024

Hi @baerwang if it's ok with you, I'm doing some minor changes on your branch to finish it

@jcamiel
Copy link
Collaborator

jcamiel commented Apr 1, 2024

/accept --force

@hurl-bot
Copy link
Collaborator

hurl-bot commented Apr 1, 2024

🕗 /accept --force is running, please wait for completion.

@hurl-bot
Copy link
Collaborator

hurl-bot commented Apr 1, 2024

✅ Pull request merged without waiting for checks and closed by jcamiel with fast forward merge..

# List of commits merged from baerwang/hurl/feat/file-limit branch into Orange-OpenSource/hurl/master branch:

@hurl-bot hurl-bot closed this Apr 1, 2024
@jcamiel jcamiel linked an issue Apr 1, 2024 that may be closed by this pull request
@jcamiel
Copy link
Collaborator

jcamiel commented Apr 1, 2024

Thanks @baerwang for this PR

@jcamiel jcamiel changed the title feat: File size limits Add --max-filesize option to limit HTTP response Apr 2, 2024
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.

Implement --max-filesize curl option
3 participants