Skip to content

Client closing an HTTP form upload crashes the server. #830

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

Closed
mangelajo opened this issue Sep 28, 2015 · 4 comments
Closed

Client closing an HTTP form upload crashes the server. #830

mangelajo opened this issue Sep 28, 2015 · 4 comments
Milestone

Comments

@mangelajo
Copy link
Contributor

I have found this while testing the web update example, if you cancel the upload of the new binary from server, and then you try to upload a new one, you get the connection open and accepted, but then it crashes on:

LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1
LmacRxBlk:1

I suspect the loop on ESP8266WebServer::_parseForm is not handling client disconnection, I tried to add a if (!client.connected()) return false inside the while(1) loop without success...

@mangelajo
Copy link
Contributor Author

Hmm, ok, the reason is that https://github.com/esp8266/Arduino/blob/esp8266/hardware/esp8266com/esp8266/libraries/ESP8266WebServer/src/Parsing.cpp#L277 uploadReadByte will lock forever in the while loop.

We should check when client.connected() goes false, and then send a UPLOAD_FILE_ABORTED or similar to the handler, so the app would be able to cleanup.

@Links2004
Copy link
Collaborator

can you try:

uint8_t ESP8266WebServer::_uploadReadByte(WiFiClient& client){
  int res = client.read();
  if(res == -1){
    while(!client.available()) {
      yield();
      if(!client.connected()) {
          break;
      }
    }
    res = client.read();
  }
  return (uint8_t)res;
}

@mangelajo
Copy link
Contributor Author

I will send you a patch, that works, but also you have to make the loop on top of this (_parseForm) check client.connected() and exit when necessary.

@mangelajo
Copy link
Contributor Author

I sent a pull request with the patch. It's been manually tested.

@igrr igrr modified the milestone: 2.0.0 Nov 5, 2015
@igrr igrr closed this as completed Nov 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants