arduino-libraries / WiFi101

Wifi library for the Arduino WiFi 101 Shield
159 stars 128 forks source link

Not reading data from client blocks data from other clients #32

Open drp0 opened 8 years ago

drp0 commented 8 years ago

I am in the process of writing a PASV FTP client using a Mega 2560 Uploading does not present a problem and is fast. Downloading does. This includes using the PASV LIST command to obtain a directory listing. In both cases, after opening the PASV port I receive 1418 bytes and then the process stalls , with the server side PASV port still open. Occasionally 1 in 5 directory listings come through complete using void dir. I have tested on two Unix L8 servers. The full code is attached, I have opened a dmz for the Arduino WiFi101 shield IP. (Not really necessary for PASV.) The shield is allocated a fixed IP by the router.

I have re-structured the pclient read code endlessly. Strangely, using the server side STAT call to get the directory works seamlessly - this uses the ftp control port. I am not aware of any default paging methods under pasv ftp. The servers do not support block transfer.

As a (very slow) workround to get ftp download working I have coded: 1) PASV 2) Connect to passive port 4) REST 0 5) RETR filename 6) grab 1024 bytes from PASV port 7) stop passive port, flush PASV 8) PASV, then connect to new PASV port 9) REST (bytes received so far - 1) 10) goto point 5 and repeat until the port times out or the PASV port is no longer connected

void dir(String prompt) { // this version stalls at 1418 bytes, most times
  if(!useserial) return; // no point
Serial.println(F("\n**************** PASV directory request "));
Serial.print(prompt);
Serial.println(F(" ****************\n"));
digitalWrite(led, HIGH); // signal dir retrieve

echo(F("Type A")); // A for ascii, I for binary
echo(F("REST 0")); // reset server pointer to 0

unsigned int hiPort = getport();
  if (hiPort == 0) {
  Serial.println(F("PASV rejected"));
  flashled(12);
  return;
  }
pclient.flush();
  if (pclient.connect(server, hiPort)) {
  Serial.println(F("Pasv Data port connected"));
  Serial.flush();
  }else {
  Serial.println(F("Pasv Data port connection failed"));
  flashled(12);
  return;
  }

int n = 0;
boolean error = false;
unsigned long bytes = 0;

retval = echo(prompt);
  if (retval.startsWith("5")) {
    if(useserial) Serial.println(F("LIST rejected"));
  pclient.stop(); response();
  digitalWrite(led, LOW); // signal end of dir retrieve
  return;
  }

unsigned long startTime = millis();
  while (pclient.connected() ) {
    if (millis()- startTime > 2000) { // extending this time does not fix 1418b bug
    error = true;
    break;    
    }
  n = pclient.available();
    if(n > 0) {
    byte getit[n];
    pclient.read(getit, n);
      for (int i = 0; i < n; i++) Serial.write(char(getit[i]));
    bytes += n;
    startTime = millis();
    }
  }

  if (error) Serial.print(F("\nTimeout. "));

Serial.print(F("Received ")); Serial.print(bytes); Serial.println(F(" bytes"));
Serial.println(F("\nClosing Pasv port")); Serial.flush();
pclient.flush(); pclient.stop(); // does not generate a response on my server
delay(100); client.flush();
digitalWrite(led, LOW); // signal end of dir retrieve
}
unsigned int getport(){
String retval = echo(F("PASV"));
  if (retval.startsWith("5")) return 0;

// we have to change to the new port
unsigned int portarray[6];
int pos = 1 + retval.length();
char cretval[pos];
retval.toCharArray(cretval, pos);
char *str = strtok(cretval,"(,");
  for ( byte i = 0; i < 6; i++) {
  str = strtok(NULL, "(,");
  portarray[i] = atoi(str);
    if(str == NULL) return 0;
  }
unsigned int hiPort, loPort;
hiPort = portarray[4] << 8;
loPort = portarray[5] & 255;
hiPort = hiPort | loPort;
  if(useserial) {
  Serial.print(F("Data port: ")); Serial.println(hiPort);
  }
return hiPort;
}

String echo(String request){ // send request and get response
  if (useserial) Serial.println(request);
client.println(request);
String retval = response();
  if (useserial) Serial.println();
return retval;
}

String response(){ // get server response with timeout
unsigned long starttime = millis();
unsigned long lastbyte = starttime;
boolean alldone = false;
boolean gotbyte = false;
String retval = "";
int received = 0;
  do{
    if (!client.connected() ){
    return "";
    }
    while (client.available() > 0 ) {
    char c = client.read();
    received ++;
      if (received < 127) retval = retval + String(c); // limit the string size
      if (useserial) Serial.write(c);
    lastbyte = millis();
    gotbyte = true;
    }
    // a pause of 200 mS second probably indicates message over
    if (gotbyte && (millis() - lastbyte > 200)) alldone = true;
  } while ( (millis() - starttime < timeout) && (!alldone));
return retval;
}`

FTPpassive2.zip

David

sandeepmistry commented 8 years ago

@drp0 thanks for providing sample code for this issue. Unfortunately I don't have access to a FTP to test it out with.

One thing to keep in mind for your next code sample, is make to code to reproduce the issue as minimal as possible, it helps us narrow down the issue. For example, the SD card code and possibly NTP code could have been removed. This way anyone trying to run your sample code the reproduce the issue doesn't need to setup an SD card shield.

Based on @ThibaultRichard's comment from https://github.com/arduino-libraries/WiFi101/pull/5#commitcomment-14252712: The main FTP client client might be receiving data while you are processing data from pclient. So internals of library will not process data from client and this will block the pclient from receiving data.

Some suggestions to try out:

drp0 commented 8 years ago

Well done Sandeep! Both suggestions work- however disconnecting the client is certainly not protocol behaviour. The client was in fact sending a string of characters, which could mean anything. I have read the ftp protocol https://www.w3.org/Protocols/rfc959/ and can not determine why the UNIX ftp servers are sending this whilst a pasv transfer is in play. The solution to pasv transfer is therefore while connected to the passive port: while(client.available()) client.read();

Cheers, David

drp0 commented 8 years ago

For reference, pasv directory:

boolean useserial = true;
unsigned long timeout = 30000; // maximum wait for ftp server response
String retval;

void dir(String prompt) { 
  if(!useserial) return;
Serial.print(F("\n**************** PASV directory request "));
Serial.print(prompt);
Serial.println(F(" ****************\n"));

echo(F("Type A")); // A for ascii, I for binary

unsigned int hiPort = getport();
  if (hiPort == 0) {
  Serial.println(F("PASV rejected"));
  return;
  }
pclient.flush();
  if (!pclient.connect(server, hiPort)) {
  Serial.println(F("Pasv Data port connection failed"));
  return;
  }

int n = 0;
boolean error = false;
unsigned long bytes = 0;

retval = echo(prompt);
  if (retval.startsWith("5")) {
    if(useserial) Serial.println(F("LIST rejected"));
  pclient.stop(); response(1000);
  return;
  }

unsigned long startTime = millis();
  while (pclient.connected() ) {
    while(client.available()) client.read();

    if (millis()- startTime > 5000) {
    Serial.print(F("\n\nTimeout. "));
    error = true;
    break;
    }
  n = pclient.available();
    if(n > 0) {
      for (int i = 0; i < n; i++) Serial.write(pclient.read());
    bytes += n;
    startTime = millis();
    }
  }
  if (pclient.connected() ) {
  Serial.println(F("PASV port connected."));
  pclient.flush(); pclient.stop();  
  response(500); // flush client to be sure
  }else{
  Serial.println(F("\nPASV has disconnected."));
  pclient.stop(); discard(100);
  }
Serial.print(F("Received ")); Serial.print(bytes); Serial.println(F(" bytes\n"));
Serial.flush();
}

unsigned int getport(){
retval = echo(F("PASV"));
  if (retval.startsWith("5")) return 0;

// we have to change to the new port
unsigned int portarray[6];
int pos = 1 + retval.length();
char cretval[pos];
retval.toCharArray(cretval, pos);
char *str = strtok(cretval,"(,");
  for ( byte i = 0; i < 6; i++) {
  str = strtok(NULL, "(,");
  portarray[i] = atoi(str);
    if(str == NULL) return 0;
  }
unsigned int hiPort, loPort;
hiPort = portarray[4] << 8;
loPort = portarray[5] & 255;
hiPort = hiPort | loPort;
  if(useserial) {
  Serial.print(F("Data port: ")); Serial.println(hiPort);
  }
return hiPort;
}

String echo(String request){ // send request and get response
  if (useserial) Serial.println(request);
client.println(request);
retval = response(timeout);
return retval;
}

String response(int tout){ // get server response with timeout
unsigned long starttime = millis();
unsigned long lastbyte = starttime;
boolean alldone = false;
boolean gotbyte = false;
retval = "";
int received = 0;
  do{
    if (!client.connected() ){
    return "";
    }
    while (client.available() > 0 ) {
    char c = client.read();
    received ++;
      if (received < 127) retval = retval + String(c); // limit the string size
      if (useserial) Serial.write(c);
    lastbyte = millis();
    gotbyte = true;
    }
    // a pause of 250 mS second probably indicates message over
    if (gotbyte && (millis() - lastbyte > 250)) alldone = true;
  } while ( (millis() - starttime < tout) && (!alldone));
return retval;
}

void discard(int quit) { // discard client bytes
unsigned long lastmsg = millis();  
  do{
    if (!client.connected()) return;
    while (client.available() ) client.flush();
    } while ( millis() - lastmsg < quit);
}
drp0 commented 8 years ago

Upload:

byte led = 12; // choose carefully - some ports conflict with wifi101
               // not 5, 7 or 10
#include <SdFat.h>
SdFat sd; // file system object
Sd2Card card;
SdFile sdfile;
#define BUF_SIZE 1024 // 512 b optimized buffer for arduino flash transfer
byte buff[BUF_SIZE];

boolean ftpUpload(String sentfile) {
char fileout[1 + sentfile.length()]; // buffer for filename
  if(useserial) Serial.println(F("\n**************** FTP Upload ****************\n"));

  if(sentfile == "") {
    if(useserial) Serial.println(F("No file, nothing to do!\n"));
  noretry = true;
  flashled(2);
  return false;
  }
sentfile.toCharArray(fileout, 1 + sentfile.length());
  //if (! sdfile.open(&root, fileout, O_READ)) { // offset from root: old sdfat
  if (! sdfile.open(fileout, O_READ)) {  // current directory
    if (useserial)Serial.println(F("Arduino File failed to open, nothing to do!\n"));
  flashled(2);
  return false;
  }
unsigned long filesize = sdfile.fileSize();
  if(filesize == 0) {
    if (useserial)Serial.println(F("Arduino file has zero size, nothing to do!\n"));
  flashled(2);
  return false;  
  }
digitalWrite(led, HIGH); // signal ftp out

unsigned int hiPort = getport();
  if (hiPort == 0) {
  quit(F("PASV rejected"), 12);
  return false;
  }

  if (pclient.connect(server, hiPort)) {
    if(useserial) Serial.println(F("Pasv Data port connected"));
  }else {
  quit(F("Passive Data port connection failed"), 12);
  return false;
  }  

echo(F("TYPE I")); // binary
echo(F("REST 0")); // reset server pointer to 0
pclient.flush();

retval = echo("STOR " + sentfile); // (Non passive would require a port command first)
  if (retval.startsWith("5")) {
    if(useserial) Serial.println(F("Data port disconnected"));
  pclient.stop(); response(1000);    
  quit(F("STOR rejected"), 14);
  return false;
  }

// now send the data file!
  if (useserial){
  Serial.print(F("Uploading file ")); Serial.print(sentfile + " ");
  Serial.print(filesize); Serial.println(F(" bytes"));
  }
// Use fast optimized block sd write, block server read

  while (filesize >= BUF_SIZE){
    while(client.available()) client.read();
  sdfile.read(buff, BUF_SIZE);
  pclient.write(buff, BUF_SIZE);
  filesize = filesize - BUF_SIZE;
  }
  if (filesize > 0){
  sdfile.read(buff, filesize);
  pclient.write(buff, filesize);
  }

  if (useserial)Serial.println(F("File upload done"));
sdfile.close(); 
  if(useserial) Serial.println(F("Data port disconnected"));
pclient.stop(); response(1000);    
  if (useserial) {
  echo("STAT " + sentfile);   // single file info non PASV
  //echo("STAT -a -l");       // current directory non PASV
  //dir("LIST " + sentfile ); // single file info PASV  
  //dir(F("LIST -a -l") );    // Current directory, PASV
  }
return true;
}

void quit(String prompt, byte flash){
  if (flash > 0) prompt = "Error " + prompt;
  if (useserial) Serial.println(prompt);
echo(F("QUIT"));
client.stop();
  if (flash > 0){
  sdfile.close();  
    if (useserial) Serial.println(F("FTP failed"));
  flashled(flash);   
  }
}

void flashled(byte repeat){
delay(1000);
  for(byte i = 0; i < repeat; i++) {
  digitalWrite(led, LOW);
  delay(500);
  digitalWrite(led, HIGH);
  delay(500);
  }
digitalWrite(led, LOW); 
}
drp0 commented 8 years ago

Download:

boolean ftpDownload(String recfile) {
unsigned int hiPort;
char infile[1 + recfile.length()]; // buffer for filename
  if(useserial) Serial.println(F("\n**************** FTP Download ****************\n"));

  if(recfile == "") {
    if(useserial) Serial.println(F("No file, nothing to do!\n"));
  flashled(2);
  return false;
  }
recfile.toCharArray(infile, 1 + recfile.length());
  if(sd.exists(infile)){ // remove infile if present
    if(!sd.remove(infile) ) {
      if (useserial) Serial.println(F("Download file on Arduino exists and will not delete\n"));  
    flashled(2);
    return false;
    }
  }
digitalWrite(led, HIGH); // signal ftp download
ok = sdfile.open(infile, O_CREAT | O_WRITE);
  if (!ok ) {
    if (useserial) Serial.println(F("Quitting. Arduino File failed to open.\n"));  
  flashled(2);
  return false;
  }

useserial = false;
echo("STAT"); // get status to clear old messages
pclient.flush(); // make sure this port is clear
  if (pclient.connected() ) {
  pclient.stop();
  response(2000);
  pclient.flush();
  }
useserial = olduseserial;

echo(F("TYPE I")); // binary

long remotesize = 0;
retval = echo("SIZE " + recfile);
  if (retval.startsWith("5")) {
  quit(F("SIZE rejected (File not found?)"), 14);
  return false;
  } else {
  byte p = 1 + retval.indexOf(" ");
  retval = retval.substring(p);
  retval.trim();
  remotesize = retval.toFloat();
    if (remotesize == 0) {
    noretry = true;
      quit(F("Server download file has zero size"), 14);
    return false;    
    }
  }

hiPort = getport();
  if (hiPort == 0) {
  quit(F("PASV rejected"), 12);
  return false;
  }

  if (pclient.connect(server, hiPort)) {  // server
    if(useserial) Serial.println(F("Pasv Data port connected"));
  }else {
  quit(F("Passive Data port connection failed"), 12);
  return false;
  }  

// now receive the data file!
  if (useserial){
  Serial.print(F("Downloading file ")); Serial.println(recfile);
  Serial.flush();
  }

retval = echo(F("REST 0")); // reset server pointer to 0
  if (retval.startsWith("5")) { // error code returned
  pclient.stop(); discard(250);
  quit(F("REST rejected"), 16);
  return false;
  }

retval = echo("RETR " + recfile); // (Non passive would require a port command first)
  if (retval.startsWith("5")) {
    if(useserial) Serial.println(F("Data port disconnected"));
  pclient.stop(); discard(250);    
  quit(F("RETR rejected"), 14);
  return false;
  }

unsigned long filesize = 0;
boolean error = false;
unsigned long lastbyte = millis();
int n;
  while(pclient.connected()) {
    while(client.available()) client.read();
  n = pclient.available();
     if(n > 0 ){
      while (n >= BUF_SIZE) {
      pclient.read(buff, BUF_SIZE);
      sdfile.write(buff, BUF_SIZE);
      filesize = filesize + BUF_SIZE;
      n = n - BUF_SIZE;                    
      }

      if (n > 0) {
      pclient.read(buff, n);
      sdfile.write(buff, n);
      filesize = filesize + n;
      }
    lastbyte = millis();
    }

    if (millis() - lastbyte > 10000) {
    error = true;
    break;
    }
  }

ok = false;
  if (filesize == remotesize) ok = true;

  if (useserial) {
    if (error) {
    Serial.print(F("\nTimeout. "));
    }else{
    Serial.print(F("\nFile download complete. "));
    }
  Serial.print(filesize); Serial.print(F(" bytes: "));
    if (ok) Serial.println(F("SUCCESS")); else Serial.println(F("FAILURE"));
  Serial.println(F("\nData port disconnecting.."));
  Serial.flush();
  }
pclient.flush();
  if (pclient.connected() ){
  pclient.stop();
  response(1000);
  pclient.flush();
  } 
sdfile.close();
  if(useserial){
  Serial.print(F("\n**************** Aduino "));
  Serial.print(local); Serial.println(F(" ****************\n"));
  // list root files with date and size
  sd.ls(LS_DATE | LS_SIZE);
  Serial.println(F("\n******************************************\n"));
  Serial.flush();
  }
digitalWrite(led, LOW); // signal end of send
  if(ok) return true; else return false;
}
sandeepmistry commented 8 years ago

@drp0 great, thanks for trying out my suggestion!

I'm going to rename this issue and provide some a minimal test case on how to reproduce.

sandeepmistry commented 8 years ago

Simple way to reproduce:

#include <WiFi101.h>

char ssid[] = "yourNetwork"; //  your network SSID (name)
char pass[] = "password";    // your network password (use for WPA, or use as key for WEP)

char server[] = "10.0.1.26"; // server IP
int port = 8000;

WiFiClient client1;
WiFiClient client2;

void setup() {
  Serial.begin(9600);
  while(!Serial);

  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    // don't continue:
    while (true);
  }

  Serial.print("Attempting to connect to SSID: ");
  Serial.println(ssid);
  while (WiFi.begin(ssid, pass) != WL_CONNECTED) {
    Serial.println("Connection failed, retrying");
  }

  Serial.println("Connected to wifi");

  if (client1.connect(server, port)) {
    Serial.println("Connected to server 1");
  } else {
    Serial.println("Connection to server failed 1");
  }

  if (client2.connect(server, port)) {
    Serial.println("Connected to server 2");

    int read = 0;
    while(client1.connected()) {
      int n = client1.available();
      if(n > 0) {
        byte getit[n];
        read += client1.read(getit, n);

        Serial.print("Bytes read = ");
        Serial.println(read);
      }
    }

    Serial.println("Disconnected from server");
  } else {
    Serial.println("Connection to server failed 2");
  }
}

void loop() {
}

Node.js server:

var net = require('net');

net.createServer(function (socket) {
  console.log('client connected', socket.remoteAddress, socket.remotePort);

  setTimeout(function() {
      var data = new Buffer(15000).fill(0);

      socket.write(data, function() {
        socket.end();
      });
  }, 10000);
}).listen(8000);

Typically, only 2892 bytes are read from client1 before the sketch stops, even though the server is sending 15000 bytes to both client1 and client2.

sandeepmistry commented 6 years ago

Now that #204 is merged, this will no longer happen on SAMD boards like the MKR1000. However, it will still happen on AVR boards when a packet over 64 bytes is received.