Conversation
Verified with test 3100 Fixes #4750 Closes #....
|
Once more a comment on closed PR... Seems to make sense though |
|
Agreed. It's even strange that it can update the first host... |
Suggested-by: Erik Janssen URL: #9870 (comment)
Suggested-by: Erik Janssen URL: #9870 (comment) Closes #9882
| * safe assumption as no other redirects should happen from RTSP. | ||
| */ | ||
| if(conn->handler->protocol & CURLPROTO_RTSP) | ||
| data->set.redir_protocols = CURLPROTO_RTSP; |
There was a problem hiding this comment.
This probably breaks reuse of handle with another protocol.
If handle is used for RTSP then for HTTP, redirection won't work on HTTP unless reset externally between both uses.
I would rather use a copy of set.redir_protocols into a new state.redir_protocols field in the whole library.
There was a problem hiding this comment.
Yikes, that is indeed a good remark. I need to fix this...
There was a problem hiding this comment.
The RTSP code also wrongly sets data->set.opt_no_body in several places...
There was a problem hiding this comment.
I did not see ithem before but you're right. I noticed this particular one while rebasing the sieve PR, as it also features redirection.
I've just done a quick check for more and there may be some (although not all listed):
$ find lib -name '*.c' | xargs grep -e '->set\.[A-Za-z0-9_.]* *= ' | grep -Fv 'setopt.c
> url.c'
lib/easy.c: dst->set.postfields = dst->set.str[i];
lib/easy.c: outcurl->set.buffer_size = data->set.buffer_size;
lib/http.c: data->set.redir_protocols = CURLPROTO_RTSP;
lib/ftp.c: data->set.ftp_filemethod = FTPFILE_MULTICWD;
lib/ftp.c: data->set.fwrite_func = Curl_ftp_parselist;
lib/ftp.c: data->set.out = data;
lib/ftp.c: data->set.fwrite_func = ftpwc->backup.write_function;
lib/ftp.c: data->set.out = ftpwc->backup.file_descriptor;
lib/doh.c: doh->set.fmultidone = doh_done;
lib/doh.c: doh->set.dohfor = data; /* identify for which transfer this is done */
lib/rtsp.c: data->set.opt_no_body = TRUE; /* most requests don't contain a body */
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = FALSE;
lib/rtsp.c: data->set.opt_no_body = TRUE;
lib/transfer.c: data->set.trailer_data = NULL;
lib/transfer.c: data->set.trailer_callback = NULL;
lib/transfer.c: data->set.upload = false;
lib/http2.c: node->data->set.stream_depends_on = child;
lib/http2.c: parent->set.stream_dependents = 0;
lib/http2.c: (*tail)->data->set.stream_depends_e = FALSE;
lib/http2.c: child->set.stream_depends_on = parent;
lib/http2.c: child->set.stream_depends_e = exclusive;
lib/http2.c: parent->set.stream_dependents = data->next;
lib/http2.c: child->set.stream_depends_on = 0;
lib/http2.c: child->set.stream_depends_e = FALSE;
lib/multi.c: data->state.conn_cache->closure_handle->set.timeout = data->set.timeout;
lib/conncache.c: connc->closure_handle->set.buffer_size = READBUFFER_MIN;
lib/vtls/vtls.c: data->set.general_ssl.max_ssl_sessions = amount;
$
Maybe we can plan to have only setopt values in set and r/w runtime values in state? Just an idea...
There was a problem hiding this comment.
Maybe we can plan to have only setopt values in set and r/w runtime values in state?
I think we should, yes. And then possibly invent and set up some some kind of framework/markup or something that lets us scan for mistakes like the present.
There was a problem hiding this comment.
And then possibly invent and set up some some kind of framework/markup
Suggestion:
- declare
setasconst - use a cast with a comment in the few functions allowed to alter it.
Not very elegant but will cause a compile error if such a mistake occurs again. The C language does not offer many alternatives to achieve that. And it does not require any external test or tool.
known bug 6.8