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 CURLOPT_TFTP_NO_OPTIONS for TFTP servers that ignore the option exension #481

Closed
bagder opened this issue Oct 8, 2015 · 6 comments
Closed
Assignees

Comments

@bagder
Copy link
Member

bagder commented Oct 8, 2015

Some TFTP server implementations ignore the "TFTP Option extension" (RFC 1782-1784, 2347-2349), or implement it in a flawed way, causing problems with libcurl. Another switch for curl_easy_setopt "CURLOPT_TFTP_NO_OPTIONS" is introduced which prevents libcurl from sending TFTP option requests to a server, avoiding many problems caused by faulty implementations.

patch by Michael König

@jay
Copy link
Member

jay commented Nov 2, 2015

@bagder I'd like to see this feature get in if possible, I didn't realize it didn't make it in last round. I already took a look back then and suggested some changes at https://github.com/jay/curl/compare/master...jay:tftp_no_options?expand=1. You said you were going to help with a test case and I had one reservation regarding blocksize:

Now that we're not sending options (tsize, blksize, timeout), someone will need to check that there is not any code that depends on any of those being sent. As far as I can tell no but someone experienced in this protocol may want to take a look. The blocksize for example appears will default to 512 so even if a user specifies a bigger blocksize and the server doesn't return a blocksize 512 will be used.

That's probably fine, but I'm not 100%.

@mkoenig75
Copy link

@jay About your reservation:
To my knowledge there is no back channel for option information. So even if the application was using the option to change the BLKSIZE it would never know if the server agreed to it or not.

The only way is through educated guessing in the data callbacks. And since the server was never required to agree to anything i see no problem here. You always needed error handling in case the BLKSIZE was not what you expected.

If you meant code inside libcurl, i did not encounter anything myself.

@bagder
Copy link
Member Author

bagder commented Nov 6, 2015

I just pushed commit 60c8629, which makes all TFTP test cases verify that the exact set of options is passed to the server so checking for a lack of those options is now also possible.

@jay jay self-assigned this Nov 6, 2015
@jay
Copy link
Member

jay commented Jan 7, 2016

@bagder Where are we with the feature window? I can't find a notice it will be closed. I have some time tomorrow and I'd like to land this, it just needs a test. Also I'd like to get in CURLINFO_TLS_SSL_PTR, I relent on my bikeshedding over the name.

@bagder
Copy link
Member Author

bagder commented Jan 7, 2016

ouch, I completely forgot to post a note about it. But as you probably felt, we're beyond that now for 7.47.0 - release date is set for Jan 27th. (I'll send an email now)

jay pushed a commit that referenced this issue Feb 23, 2016
Some TFTP server implementations ignore the "TFTP Option extension"
(RFC 1782-1784, 2347-2349), or implement it in a flawed way, causing
problems with libcurl. Another switch for curl_easy_setopt
"CURLOPT_TFTP_NO_OPTIONS" is introduced which prevents libcurl from
sending TFTP option requests to a server, avoiding many problems caused
by faulty implementations.

Bug: #481
jay added a commit that referenced this issue Feb 23, 2016
- Add tests.

- Add an example to CURLOPT_TFTP_NO_OPTIONS.3.

- Add --tftp-no-options to expose CURLOPT_TFTP_NO_OPTIONS.

Bug: #481
@jay
Copy link
Member

jay commented Feb 23, 2016

@mkoenig75 Landed in 9dc3eae and 186546f. Thanks and sorry for the long delay.

@jay jay closed this as completed Feb 23, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants