Orange-OpenSource / hurl

Hurl, run and test HTTP requests with plain text.
https://hurl.dev
Apache License 2.0
12.72k stars 477 forks source link

Configure --max-time per request #3162

Open jcamiel opened 3 weeks ago

jcamiel commented 3 weeks ago

Make --max-time configurable per request:

GET https://foo.bin
[Options]
max-time: 500s
HTTP 200
lambrospetrou commented 3 weeks ago

This would be nice to have, but can we make the CLI option to be the "max" time that any inline option might specify?

I am parsing the Hurl file with hurlfmt and do validation on my side as well, but just want to see how the inline option would interact with the CLI option.

jcamiel commented 3 weeks ago

Ha, didn't see that this can be problematic with https://skybear.net! Honestly, when reviewing the change log for 5.0.0, I was surprised that we haven't implemented it yet and hesitated to label it as a bug.

Currently, option in [Options] section overrides CLI option so it would be strange to do a particular case for this one. And we have some options that can't be configured by request : --file-root, --to-entry. For these options, it makes no sens to be per request configurable. On the other side, a maximum timeout seems a reasonable option per request (I was 100% sure we could do it).

Either we change the priority order of CLI option vs [Options] (I'm not really fan of changing it as I prefer the current priority order), or we don't implement max-time per request, which is unfortunate.

Could you analyse the Hurl file and whitelist only certain option in [Options] section? I can see this being sufficient. Besides --max-time, we have current option that could be problematic if overridden by user like --proxy no?

lambrospetrou commented 3 weeks ago

Could you analyse the Hurl file and whitelist only certain option in [Options] section?

Yes, I do this right now, by rejecting specific options and values. OK, if the precedence priority is for the inline options to override the CLI then I will keep expanding that rejection list on my side.

(I remember the priority discussion in the issue about variables as well and that would be a bit different order, but maybe I remember wrong)

we have current option that could be problematic if overridden by user like --proxy no?

Hmm, I don't see this option in https://hurl.dev/docs/request.html#options

What's the best authoritative source I can always keep an eye on for inline options allowed in case I need to reject when upgrading Hurl versions?

edit: Is this the best list? https://github.com/Orange-OpenSource/hurl/blob/master/packages/hurl_core/src/parser/option.rs#L42

jcamiel commented 3 weeks ago

Yes, you can find it also in the grammar but we've hand coded the parser from the grammar so the grammar may not be "complete" (see https://github.com/Orange-OpenSource/hurl/blob/master/docs/spec/grammar/hurl.grammar). The parser code is the best place.