#820 TFTP file transfer issue in Curl 7.19.4

closed-fixed
TFTP (14)
5
2013-06-21
2009-03-31
Vijay G
No

In Curl lib 7.19.4, tftp_tx() is not handling the case for TFTP_EVENT_OACK. This case should be added in tftp_tx(). In tftp_rx(), this case is already handled.
In RFC 2348,it is given as "If the server is willing to accept the blocksize option, it sends an Option Acknowledgment (OACK) to the client."
I was not able to upload files, as we are not handling this scenario.

Discussion

1 2 > >> (Page 1 of 2)
  • Vijay G
    Vijay G
    2009-03-31

    The patch for this issue is

    /**********************************************************
    *
    * tftp_tx
    *
    * Event handler for the TX state
    *
    **********************************************************/
    static CURLcode tftp_tx(tftp_state_data_t *state, tftp_event_t event)
    {
    struct SessionHandle *data = state->conn->data;
    int sbytes;
    int rblock;
    CURLcode res = CURLE_OK;
    struct SingleRequest *k = &data->req;

    switch(event) {

    case TFTP_EVENT_ACK:
    /* Ack the packet */
    rblock = getrpacketblock(&state->rpacket);

    if(rblock != state->block) {
    /* This isn't the expected block. Log it and up the retry counter */
    infof(data, "Received ACK for block %d, expecting %d\n",
    rblock, state->block);
    state->retries++;
    /* Bail out if over the maximum */
    if(state->retries>state->retry_max) {
    failf(data, "tftp_tx: giving up waiting for block %d ack",
    state->block);
    res = CURLE_SEND_ERROR;
    }
    else {
    /* Re-send the data packet */
    sbytes = sendto(state->sockfd, (void *)&state->spacket,
    4+state->sbytes, SEND_4TH_ARG,
    (struct sockaddr *)&state->remote_addr,
    state->remote_addrlen);
    /* Check all sbytes were sent */
    if(sbytes<0) {
    failf(data, "%s", Curl_strerror(state->conn, SOCKERRNO));
    res = CURLE_SEND_ERROR;
    }
    }
    return res;
    }
    /* This is the expected packet. Reset the counters and send the next
    block */
    state->block++;
    state->retries = 0;
    setpacketevent(&state->spacket, TFTP_EVENT_DATA);
    setpacketblock(&state->spacket, state->block);
    if(state->block > 1 && state->sbytes < state->blksize) {
    state->state = TFTP_STATE_FIN;
    return CURLE_OK;
    }
    res = Curl_fillreadbuffer(state->conn, state->blksize,
    (int *)&state->sbytes);
    if(res)
    return res;
    sbytes = sendto(state->sockfd, (void *)state->spacket.data,
    4+state->sbytes, SEND_4TH_ARG,
    (struct sockaddr *)&state->remote_addr,
    state->remote_addrlen);
    /* Check all sbytes were sent */
    if(sbytes<0) {
    failf(data, "%s", Curl_strerror(state->conn, SOCKERRNO));
    return CURLE_SEND_ERROR;
    }
    /* Update the progress meter */
    k->writebytecount += state->sbytes;
    Curl_pgrsSetUploadCounter(data, k->writebytecount);
    break;
    case TFTP_EVENT_OACK:
    /* This is the expected packet. Reset the counters and send the next
    block */
    state->block++;
    state->retries = 0;
    setpacketevent(&state->spacket, TFTP_EVENT_DATA);
    setpacketblock(&state->spacket, state->block);
    if(state->block > 1 && state->sbytes < state->blksize) {
    state->state = TFTP_STATE_FIN;
    return CURLE_OK;
    }
    res = Curl_fillreadbuffer(state->conn, state->blksize,
    (int *)&state->sbytes);
    if(res)
    return res;
    sbytes = sendto(state->sockfd, (void *)state->spacket.data,
    4+state->sbytes, SEND_4TH_ARG,
    (struct sockaddr *)&state->remote_addr,
    state->remote_addrlen);
    /* Check all sbytes were sent */
    if(sbytes<0) {
    failf(data, "%s", Curl_strerror(state->conn, SOCKERRNO));
    return CURLE_SEND_ERROR;
    }
    /* Update the progress meter */
    k->writebytecount += state->sbytes;
    #ifndef CURL_DISABLE_PROGRESS
    Curl_pgrsSetUploadCounter(data, k->writebytecount);
    #endif

    break;

    case TFTP_EVENT_TIMEOUT:
    /* Increment the retry counter and log the timeout */
    state->retries++;
    infof(data, "Timeout waiting for block %d ACK. "
    " Retries = %d\n", state->retries);
    /* Decide if we've had enough */
    if(state->retries > state->retry_max) {
    state->error = TFTP_ERR_TIMEOUT;
    state->state = TFTP_STATE_FIN;
    }
    else {
    /* Re-send the data packet */
    sbytes = sendto(state->sockfd, (void *)state->spacket.data,
    4+state->sbytes, SEND_4TH_ARG,
    (struct sockaddr *)&state->remote_addr,
    state->remote_addrlen);
    /* Check all sbytes were sent */
    if(sbytes<0) {
    failf(data, "%s", Curl_strerror(state->conn, SOCKERRNO));
    return CURLE_SEND_ERROR;
    }
    /* since this was a re-send, we remain at the still byte position */
    Curl_pgrsSetUploadCounter(data, k->writebytecount);
    }
    break;

    case TFTP_EVENT_ERROR:
    state->state = TFTP_STATE_FIN;
    break;

    default:
    failf(data, "%s", "tftp_tx: internal error");
    break;
    }

    return res;
    }

     
  • Vijay G
    Vijay G
    2009-03-31

    Ignore the line #ifndef CURL_DISABLE_PROGRESS.

    I have added the case statement for TFTP_EVENT_OACK to fix the issue.

     
  • Please make the patch with 'diff -u' and post that here instead of a full code snippet.

     
  • Vijay G
    Vijay G
    2009-04-01

    This is the patch created using ""diff -u"

    --- tftp_1.80.c 2009-03-25 09:34:33.007209600 +0530
    +++ tftp.c 2009-04-01 15:05:48.895218000 +0530
    @@ -723,6 +723,37 @@
    Curl_pgrsSetUploadCounter(data, k->writebytecount);
    break;

    +case TFTP_EVENT_OACK:
    + /* This is the expected packet. Reset the counters and send the next
    + block */
    + state->block++;
    + state->retries = 0;
    + setpacketevent(&state->spacket, TFTP_EVENT_DATA);
    + setpacketblock(&state->spacket, state->block);
    + if(state->block > 1 && state->sbytes < state->blksize) {
    + state->state = TFTP_STATE_FIN;
    + return CURLE_OK;
    + }
    + res = Curl_fillreadbuffer(state->conn, state->blksize,
    + (int *)&state->sbytes);
    + if(res)
    + return res;
    + sbytes = sendto(state->sockfd, (void *)state->spacket.data,
    + 4+state->sbytes, SEND_4TH_ARG,
    + (struct sockaddr *)&state->remote_addr,
    + state->remote_addrlen);
    + /* Check all sbytes were sent */
    + if(sbytes<0) {
    + failf(data, "%s", Curl_strerror(state->conn, SOCKERRNO));
    + return CURLE_SEND_ERROR;
    + }
    + /* Update the progress meter */
    + k->writebytecount += state->sbytes;
    + Curl_pgrsSetUploadCounter(data, k->writebytecount);
    +
    +break;
    +
    +
    case TFTP_EVENT_TIMEOUT:
    /* Increment the retry counter and log the timeout */
    state->retries++;

     
  • Vijay G
    Vijay G
    2009-04-01

     
    Attachments
  • Vijay G
    Vijay G
    2009-04-10

    Daniel,

    Can I know when these changes will be merged to the tftp.c file

     
  • Any chance you can write up a test case for the test suite that shows the problem and thus how this fix cures it?

    I don't see any TFTP expert commenting on this patch and I personally know little about these details and I haven't had the time to read up so it would help a lot.

     
  • Vijay G
    Vijay G
    2009-04-17

    Without this fix I was not able to upload any files. That is the only test case I can think of.

     
  • Test case 285 and 286 already do TFTP uploads and they've worked for a long time. So no, "just uploading" is not a proper test case. It clearly requires something more special than so...

     
  • So there's no chance you can help us make the test suite test for this? I'm still reluctant to add this without understanding it further.

     
1 2 > >> (Page 1 of 2)