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

Does ESP8266WebServer and ESP8266WiFi now require non32xfer to be enabled? #7928

Closed
blue-wind-25 opened this issue Mar 17, 2021 · 9 comments · Fixed by #7935 or #7951
Closed

Does ESP8266WebServer and ESP8266WiFi now require non32xfer to be enabled? #7928

blue-wind-25 opened this issue Mar 17, 2021 · 9 comments · Fixed by #7935 or #7951
Assignees
Milestone

Comments

@blue-wind-25
Copy link

Note: copied from my comment in #6979 as per @earlephilhower suggestion.

Hello, I would like to confirm my founding. It seems that the updated ESP8266WebServer (and maybe ESP8266WiFi) relies on the non32xfer service because:

template <typename ServerType>
void ESP8266WebServerTemplate<ServerType>::send_P(int code, PGM_P content_type, PGM_P content) {
StreamConstPtr ref(content, strlen_P(content));
return send(code, String(content_type).c_str(), &ref);
}


Then, because PGM_P is const char* then this CTOR will be called:
StreamConstPtr(const char* buffer, size_t size)
: _buffer(buffer), _size(size), _byteAddressable(__byteAddressable(buffer)) { }

which mean that the buffer will be read using non PGM method.


Also, there is another CTOR:
StreamConstPtr(const __FlashStringHelper* text)
: _buffer(reinterpret_cast<const char*>(text)), _size(strlen_P((PGM_P)text)), _byteAddressable(false) { }

It casts __FlashStringHelper to const char*. While strln_P() is correctly called, the buffer will be read using non PGM method.


Then pgmspace.h still defines PSTR as:

#ifndef PSTRN
#define PSTRN(s,n) (__extension__({static const char __pstr__[] __attribute__((__aligned__(n))) __attribute__((section( "\".irom0.pstr." __FILE__ "." __STRINGIZE(__LINE__) "." __STRINGIZE(__COUNTER__) "\", \"aSM\", @progbits, 1 #"))) = (s); &__pstr__[0];}))
#endif
#ifndef PSTR
#define PSTR(s) PSTRN(s,PSTR_ALIGN)
#endif

So it seems that PSTR strings still need to be accessed using the _P() functions.


I found that if I do not enable/include the non32xfer service then calls to web server send_P() will cause exception 3 (EXCCAUSE_LOAD_STORE_ERROR) inside the Arduino String class when copying the characters.

It seems also the case for SSL web server (even with non32xfer enabled, my SSL web server failed to send anything to the browser despite it does not cause exception 3). I am still unsure why this behavior with SSL happened.

I want to ask if this was by design (non32xfer must be enabled/included)?

I need to inform you that until at least the new official Arduino package is released, I will not be able to produce a test sketch because like I said to @earlephilhower in my other discussion, I do not use the Arduino build system.

Because I also follow updates from Arduino ESP8266 GIT and then modify the cores a bit to match my need and style, my application code will not compile using Arduino's original core. Therefore, I am also not sure if my comment about this is actually valid when using the original core and Arduino IDE. Thank you!

@d-a-v
Copy link
Collaborator

d-a-v commented Mar 23, 2021

@blue-wind-25 Thanks for reporting !
I have been trying to reproduce and I was about to give up and ask for an MCVE just before I discovered this byte-by-byte flash transfer which was not covered in StreamConstPtr. I reckon I have mainly been testing the block transfers.
Could you please have a try with #7935 and report ?

@blue-wind-25
Copy link
Author

@d-a-v You are welcome!

Yes, #7935 fix the exception 3 problem. However, it seems that secure web server still does not work properly.

It does enter the on() handler because I can see my debugging message on the UART, however send(200, "text/plain", "OK"); does not send anything back to the browser. The send() does return and does not crash because I print to the UART after send(). Maybe not enough timeout because SSL is slower than non-SSL?

By my observation, the new ESP8266WiFi library also use the new Stream, so I do not think the problem is in the new Stream anymore because if I use the old ESP8266WebServer library (with the new ESP8266WiFi and #7935), secure web server works.

On the other hand, if I use only the new ESP8266WebServer (or both the new ESP8266WebServer and the new ESP8266WiFi), SSL does not work. Maybe somehow the timeout mechanism has changed and does not work with SSL?

About:

//if (_byteAddressable)
// return _peekPointer < _size ? _buffer[_peekPointer++] : -1;
return _peekPointer < _size ? pgm_read_byte(&_buffer[_peekPointer++]) : -1;

As far as I know the PGM function can be used both to read from RAM and flash because ESP8266 flash and RAM are in one address space, the difference is that reading from flash must be 32-bit alligned. So, how much overhead does the PGM function when used for reading for RAM? If it is negligible, then I guess always using pgm_read_byte() will be fine.

I'll try to help finding why the new ESP8266WebServer does not work with SSL. However, because I customized the core and the whole build system, I may not be able to produce MCVE that will compile using Arduino. But I will update my customized web sever library bit by bit to follow the Arduino GIT, maybe I can find which function that cause SSL to block.

Thank you for your help.

@blue-wind-25
Copy link
Author

blue-wind-25 commented Mar 23, 2021

@d-a-v I found another bug.

In library ESP8266WiFi
Inside ClientContext.h
Function bool _write_some()

In line 534 there is const char* buf = _datasource->peekBuffer();

Then in line 549 err_t err = tcp_write(_pcb, buf, next_chunk_size, flags);

When I handle the form for POST request and then using send_P() to send the form HTML because the data is from PGM string, the code above bypassed the read byte logic in StreamConstPtr and because tcp_write() cannot handle data from PGM, exception 3 will occur again.

In my case, I can do something like this to fix:

std::unique_ptr<char[]> tmp = new char[ tcp_sndbuf(_pcb) ];
_datasource->readBytes(tmp.get(), next_chunk_size);
err_t err = tcp_write(_pcb, tmp.get(), next_chunk_size, flags);

Of course this line must be commented-out:
//_datasource->peekConsume(next_chunk_size);

But I am not sure if doing that is a good idea because we are allocating and deallocating memory for every function call.

@blue-wind-25
Copy link
Author

blue-wind-25 commented Mar 23, 2021

Hello, I have an additional information. The problem with SSL seems originating from ESP8266WebServer-impl.h. Currently, in my code, I have both old and new method of sending data selectable using #if.

I still not able to find which function actually block SSL, however, I does get WDT reset the last time (using my modified code not Arduino).

I will try to narrow down the problem tomorrow.

@blue-wind-25
Copy link
Author

blue-wind-25 commented Mar 24, 2021

@d-a-v I think the error is not inside ESP8266WebServer-impl.h.

After some tracing, I found that for secure web server, in StreamSend.cpp, in function Stream::SendGenericPeekBuffer(Print* to, ...), the statement size_t w = to->availableForWrite(); is always zero.

The reason is because in WiFiClient.cpp, function WiFiClient::availableForWrite (), the _client variable is nullptr, hence _client->availableForWrite() is never called (and zero is always returned).

However if I change the content of SendGenericPeekBuffer() to:

const size_t pa = peekAvailable();
to->write( peekBuffer(), pa );
return pa;

All work fine.

This does not make sense, until I found out that WiFiClient::availableForWrite() is also called by int WiFiClientSecureCtx::_run_until(unsigned target, bool blocking) in WiFiClientSecureBearSSL.cpp. If availableForWrite() is called from inside _run_until() it will return non zero.

I think might has something to do with how browser negotiate SSL which cause connected() to never return true if the SSL negotiation is not complete yet, therefore this block:

size_t w = to->availableForWrite();
if (w == 0 && !to->outputCanTimeout())
{
// no more data can be written, ever
break;
}

thinks that no more data can be written because to->outputCanTimeout() is always false and thus no data is sent to the browser.

Commenting-out the block above also does not work because I think when inside SendGenericPeekBuffer(), WiFiClientSecureCtx::_run_until() and WiFiClientSecureCtx::_write() will never be executed, and thus the SSL handshake will never be complete.

It become make sense because SSL handshake is somehow got completed in WiFiClientSecureCtx::_write(), therefore calling to->write( peekBuffer(), pa ); directly will eventually calls WiFiClientSecureCtx::_write() and finished the SSL handshake.

I tried to add to->flush() on top of SendGenericPeekBuffer(). It still does not work.

@d-a-v
Copy link
Collaborator

d-a-v commented Mar 24, 2021

@blue-wind-25 availableForWrite is missing in SSL client. I added one which solves the issue with the HelloServerBearSSL example. I'll probably have to update it but it is worth testing until then.

I'll check on your other reports, there are interesting insights, thanks for digging this up !

@blue-wind-25
Copy link
Author

@d-a-v No problem, you are welcome!

Yes, now secure web server works in my case. Thank you!

@d-a-v
Copy link
Collaborator

d-a-v commented Mar 25, 2021

Reopening because of @blue-wind-25 's findings in ClientContext and flash/iram input Stream

@blue-wind-25
Copy link
Author

@d-a-v #7951 does fix it. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment