Configure --max-time per request

[jcamiel]

Make --max-time configurable per request:

GET https://foo.bin
[Options]
max-time: 500s
HTTP 200

[lambrospetrou]

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]

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]

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]

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.

[bp7968h]

Hi @jcamiel, is is ok if I just create to limit the max-limit to like u16 meaning can I create a new type let's say MaxTime(u16) here

[jcamiel]

Hi @bp7968h

I think the better model should be MaxTime(DurationOption), this way we can support without a lot of changes something like max-time: 500s or max-time: {{variable}}. I've not tested it but I think you can reuse DurationOption type, in theory ☺️...

[ashishajr]

Hey @jcamiel, I'd love to take this up next. This would be similar to the changes for #3163, right?

[jcamiel]

Hi @ashishajr yes exactly! I'm assigning it to you

[ashishajr]

Hey @jcamiel, I run into some trouble with this and was hoping you could help me out here

I assumed that max-time would be the same option in curl, similar to connect-timeout, but when I run a run file with the max-time option

hurl --max-time 10 --connect-timeout 2 --very-verbose expect.hurl

I get the following output

* ------------------------------------------------------------------------------
* Executing entry 1
*
* Cookie store:
*
* Request:
* POST http://localhost:8000/expect
* Expect: 100-continue
*
* Request can be run with the following curl command:
* curl --header 'Expect: 100-continue' --header 'Content-Type:' --data 'data' --connect-timeout 2 --timeout 10 'http://localhost:8000/expect'
...
...

So I assume the max-time is the timeout here but there is no --timeout option for curl

In this page for timeouts, https://everything.curl.dev/usingcurl/timeouts.html, there are only two options given --max-time and --connect-timeout. I do not see the --timeout option even in the manpage for curl or in my terminal when I run curl help all

     --suppress-connect-headers                    Suppress proxy CONNECT response headers
     --tcp-fastopen                                Use TCP Fast Open
     --tcp-nodelay                                 Set TCP_NODELAY
 -t, --telnet-option <opt=val>                     Set telnet option
     --tftp-blksize <value>                        Set TFTP BLKSIZE option
     --tftp-no-options                             Do not send any TFTP options
 -z, --time-cond <time>                            Transfer based on a time condition
     --tls-earlydata                               Allow use of TLSv1.3 early data (0RTT)
     --tls-max <VERSION>                           Maximum allowed TLS version
     --tls13-ciphers <list>                        TLS 1.3 cipher suites to use
     --tlsauthtype <type>                          TLS authentication type
     --tlspassword <string>                        TLS password
     --tlsuser <name>                              TLS username
 -1, --tlsv1                                       TLSv1.0 or greater
     --tlsv1.0                                     TLSv1.0 or greater
     --tlsv1.1                                     TLSv1.1 or greater
     --tlsv1.2                                     TLSv1.2 or greater
     --tlsv1.3                                     TLSv1.3 or greater
     --tr-encoding                                 Request compressed transfer encoding
     --trace <file>                                Write a debug trace to FILE
     --trace-ascii <file>                          Like --trace, but without hex output
     --trace-config <string>                       Details to log in trace/verbose output
     --trace-ids                                   Transfer + connection ids in verbose output
     --trace-time                                  Add time stamps to trace/verbose output
     --unix-socket <path>                          Connect through this Unix domain socket

In this file I see that the function timeout is used for parsing max-time https://github.com/Orange-OpenSource/hurl/blob/942b7d193717b54906755110821630a6cfc00ea2/packages/hurl/src/cli/options/matches.rs#L451-L454

And here the timeout function is set to the timeout option

https://github.com/Orange-OpenSource/hurl/blob/942b7d193717b54906755110821630a6cfc00ea2/packages/hurl/src/cli/options/mod.rs#L343

In this file as well, in the test, I see that the timeout option is defined but there is no reference to max-time

https://github.com/Orange-OpenSource/hurl/blob/942b7d193717b54906755110821630a6cfc00ea2/packages/hurl/src/http/curl_cmd.rs#L669-L699

Shouldn't the timeout option be replaced with max_time or have I missed something huge?

[jcamiel]

Hi @ashishajr Yes you're totally right the Hurl log is erroneous

curl --header 'Expect: 100-continue' --header 'Content-Type:' --data 'data' --connect-timeout 2 --timeout 10 'http://localhost:8000/expect'

It should be

curl --header 'Expect: 100-continue' --header 'Content-Type:' --data 'data' --connect-timeout 2 --max-time 10 'http://localhost:8000/expect'

as, effectively, there is no --timeout option. As it's "just" a debug log, I'm not surprised that we have a bug. I'm going to create an issue for it, and we'll fix it for the next release

[ashishajr]

Thanks @jcamiel, is it cool if in my PR I first change all the mentions of timeout to to max-time? This would be easier for me to make the changes and understand the code instead of first adding the max-time option and then changing timeout to max-time

[jcamiel]

We're going to change the log for sure. Regarding the name timeout I think the intention was to distinguish between the curl option name max-time and the functional name which is effectively "timeout". It's the same way we sometimes speak of location (the curl option name to follow redirection) and "follow_redirect" which is the name we use (or should use) in the code.

[ashishajr]

Ah got, that makes sense, thanks. As a couple of other options had the same name as curl's options I assumed that the options are named the same at both places.

[ashishajr]

Hey @jcamiel, I've raised a bug report for the incorrect curl command option, #4046, I'd like to take it up after I'm done with this task

[ashishajr]

Hey @jcamiel, this could be marked as done too right or do I have to make any more changes? Just wanted to confirm

[jcamiel]

Hi @ashishajr yes good catch this should be done now