Bugs item #2723219, was opened at 2009-03-31 13:55
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2723219&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: TFTP
Group: wrong behaviour
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Vijay G (vijayg23333)
Assigned to: Daniel Stenberg (bagder)
Summary: TFTP error scenario not closing connection in curl 7.19.4

Initial Comment:
In tftp.c in the curl lib 7.19.4, whenever there is any error in TFTP, it is better to close the
connection. In ftp.c, its being done in the same way. "conn->bits.close =
TRUE; " should be called in tftp_do() in if(state->error != TFTP_ERR_NONE)
to resolve this issue.

----------------------------------------------------------------------

>Comment By: Daniel Stenberg (bagder)
Date: 2009-04-17 13:28

Message:
Wow. That description makes it sound like this suggested fix is rather
painting over the real problem instead of addressing it. It should of
course not matter if the connection was re-used or not.

Can you adjust an existing test cast to make it repeat your problem?

----------------------------------------------------------------------

Comment By: Vijay G (vijayg23333)
Date: 2009-04-17 13:24

Message:
In my application, if a file is not present in the server, I was not able
to receive any other file also in the same connection. Thats the reason why
I was closing the connection.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-04-13 20:15

Message:
You can help getting this added faster by providing test cases that repeat
the problem(s) and thus showing how the proposed patch(es) work.

For this close case, shouldn't we rather only mark the connection for
closure for the specific problems that can be considered to be that "fatal"
?

 I mean, TFTP_ERR_NOTFOUND is just a bad file name. Not a really good
reason to force a closure of the connection methinks. TFTP_ERR_NORESPONSE
however sounds like a more serious problem that probably deserves a
complete close.

----------------------------------------------------------------------

Comment By: Vijay G (vijayg23333)
Date: 2009-04-10 09:30

Message:
Daniel,

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

----------------------------------------------------------------------

Comment By: Vijay G (vijayg23333)
Date: 2009-04-01 11:45

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

--- tftp_1.80.c 2009-04-01 15:08:58.939660400 +0530
+++ tftp.c 2009-04-01 15:09:42.954722000 +0530
@@ -1109,6 +1109,7 @@
 
   /* If we have encountered an error */
   if(state->error != TFTP_ERR_NONE) {
+ conn->bits.close = TRUE; /*marked for closure */
 
     /* Translate internal error codes to curl error codes */
     switch(state->error) {

----------------------------------------------------------------------

Comment By: Vijay G (vijayg23333)
Date: 2009-03-31 14:20

Message:
The below patch will fix this problem :

/**********************************************************
 *
 * tftp
 *
 * The do callback
 *
 * This callback handles the entire TFTP transfer
 *
 **********************************************************/

static CURLcode tftp_do(struct connectdata *conn, bool *done)
{
  struct SessionHandle *data = conn->data;
  tftp_state_data_t *state;
  tftp_event_t event;
  CURLcode code;
  int rc;
  struct Curl_sockaddr_storage fromaddr;
  socklen_t fromlen;
  int check_time = 0;
  struct SingleRequest *k = &data->req;

  *done = TRUE;

  /*
    Since connections can be re-used between SessionHandles, this might be
a
    connection already existing but on a fresh SessionHandle struct so we
must
    make sure we have a good 'struct TFTP' to play with. For new
connections,
    the struct TFTP is allocated and setup in the tftp_connect()
function.
  */
  Curl_reset_reqproto(conn);

  if(!conn->proto.tftpc) {
    code = tftp_connect(conn, done);
    if(code)
      return code;
  }
  state = (tftp_state_data_t *)conn->proto.tftpc;

  /* Run the TFTP State Machine */
  for(code=tftp_state_machine(state, TFTP_EVENT_INIT);
      (state->state != TFTP_STATE_FIN) && (code == CURLE_OK);
      code=tftp_state_machine(state, event) ) {

    /* Wait until ready to read or timeout occurs */
    rc=Curl_socket_ready(state->sockfd, CURL_SOCKET_BAD,
                         state->retry_time * 1000);

    if(rc == -1) {
      /* bail out */
      int error = SOCKERRNO;
      failf(data, "%s", Curl_strerror(conn, error));
      event = TFTP_EVENT_ERROR;
    }
    else if(rc==0) {
      /* A timeout occured */
      event = TFTP_EVENT_TIMEOUT;

      /* Force a look at transfer timeouts */
      check_time = 0;

    }
    else {

      /* Receive the packet */
      fromlen = sizeof(fromaddr);
      state->rbytes = (ssize_t)recvfrom(state->sockfd,
                                        (void *)state->rpacket.data,
                                        state->blksize+4,
                                        0,
                                        (struct sockaddr *)&fromaddr,
                                        &fromlen);
      if(state->remote_addrlen==0) {
        memcpy(&state->remote_addr, &fromaddr, fromlen);
        state->remote_addrlen = fromlen;
      }

      /* Sanity check packet length */
      if(state->rbytes < 4) {
        failf(data, "Received too short packet");
        /* Not a timeout, but how best to handle it? */
        event = TFTP_EVENT_TIMEOUT;
      }
      else {

        /* The event is given by the TFTP packet time */
        event = (tftp_event_t)getrpacketevent(&state->rpacket);

        switch(event) {
        case TFTP_EVENT_DATA:
          /* Don't pass to the client empty or retransmitted packets */
          if(state->rbytes > 4 &&
              ((state->block+1) == getrpacketblock(&state->rpacket))) {
            code = Curl_client_write(conn, CLIENTWRITE_BODY,
                                     (char *)state->rpacket.data+4,
                                     state->rbytes-4);
            if(code)
              return code;
            k->bytecount += state->rbytes-4;
            Curl_pgrsSetDownloadCounter(data, (curl_off_t) k->bytecount);
          }
          break;
        case TFTP_EVENT_ERROR:
          state->error = (tftp_error_t)getrpacketblock(&state->rpacket);
          infof(data, "%s\n", (const char *)state->rpacket.data+4);
          break;
        case TFTP_EVENT_ACK:
          break;
        case TFTP_EVENT_OACK:
          code = tftp_parse_option_ack(state,
                                       (const char
*)state->rpacket.data+2,
                                       state->rbytes-2);
          if(code)
            return code;
          break;
        case TFTP_EVENT_RRQ:
        case TFTP_EVENT_WRQ:
        default:
          failf(data, "%s", "Internal error: Unexpected packet");
          break;
        }

        /* Update the progress meter */
        if(Curl_pgrsUpdate(conn))
          return CURLE_ABORTED_BY_CALLBACK;
      }
    }

    /* Check for transfer timeout every 10 blocks, or after timeout */
    if(check_time%10==0) {
      time_t current;
      time(&current);
      if(current>state->max_time) {
        DEBUGF(infof(data, "timeout: %d > %d\n",
                     current, state->max_time));
        state->error = TFTP_ERR_TIMEOUT;
        state->state = TFTP_STATE_FIN;
      }
    }

  }
  if(code)
    return code;

  /* Tell curl we're done */
  code = Curl_setup_transfer(conn, -1, -1, FALSE, NULL, -1, NULL);
  if(code)
    return code;

  /* If we have encountered an error */
  if(state->error != TFTP_ERR_NONE) {
        conn->bits.close = TRUE; /*marked for closure */
    /* Translate internal error codes to curl error codes */
    switch(state->error) {
    case TFTP_ERR_NOTFOUND:
      code = CURLE_TFTP_NOTFOUND;
      break;
    case TFTP_ERR_PERM:
      code = CURLE_TFTP_PERM;
      break;
    case TFTP_ERR_DISKFULL:
      code = CURLE_REMOTE_DISK_FULL;
      break;
    case TFTP_ERR_UNDEF:
    case TFTP_ERR_ILLEGAL:
      code = CURLE_TFTP_ILLEGAL;
      break;
    case TFTP_ERR_UNKNOWNID:
      code = CURLE_TFTP_UNKNOWNID;
      break;
    case TFTP_ERR_EXISTS:
      code = CURLE_REMOTE_FILE_EXISTS;
      break;
    case TFTP_ERR_NOSUCHUSER:
      code = CURLE_TFTP_NOSUCHUSER;
      break;
    case TFTP_ERR_TIMEOUT:
      code = CURLE_OPERATION_TIMEDOUT;
      break;
    case TFTP_ERR_NORESPONSE:
      code = CURLE_COULDNT_CONNECT;
      break;
    default:
      code= CURLE_ABORTED_BY_CALLBACK;
      break;
    }
  }
  else
    code = CURLE_OK;
  return code;
}

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2723219&group_id=976
Received on 2009-04-17