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

Data loss when processing templates #571

Closed
sascha432 opened this issue Aug 11, 2019 · 12 comments
Closed

Data loss when processing templates #571

sascha432 opened this issue Aug 11, 2019 · 12 comments
Labels

Comments

@sascha432
Copy link
Contributor

sascha432 commented Aug 11, 2019

I encountered an issue when place holders get replaced by a larger chunk of data, some part of the replaced data can be missing.

It occurs inside WebResponses.cpp _fillBufferAndProcessTemplates()

In particular at this section of the code, it seems that "len" isn't updated correctly. Setting len to originalLen fixes the issue, but I do not know if it has any side effects.

  // make room for param value
  // 1. move extra data to cache if parameter value is longer than placeholder AND if there is no room to store
  if((pTemplateEnd + 1 < pTemplateStart + numBytesCopied) && (originalLen - (pTemplateStart + numBytesCopied - pTemplateEnd - 1) < len)) {

	_cache.insert(_cache.begin(), &data[originalLen - (pTemplateStart + numBytesCopied - pTemplateEnd - 1)], &data[len]);

	//2. parameter value is longer than placeholder text, push the data after placeholder which not saved into cache further to the end
	memmove(pTemplateStart + numBytesCopied, pTemplateEnd + 1, &data[originalLen] - pTemplateStart - numBytesCopied);

	// this seems to solve it in MY case!
	len = originalLen;
  
  } else if(pTemplateEnd + 1 != pTemplateStart + numBytesCopied) {

@sascha432
Copy link
Contributor Author

Here some details

The data itself

pvstr length 544 data |light:
  - name: KFCAC52DB
    platform: mqtt
    availability_topic: home/atomic_sun_v2/status
    payload_available: 1
    payload_not_available: 0
    state_topic: home/atomic_sun_v2/state
    command_topic: home/atomic_sun_v2/set
    payload_on: 1
    payload_off: 0
    brightness_state_topic: home/atomic_sun_v2/brightness/state
    brightness_command_topic: home/atomic_sun_v2/brightness/set
    brightness_scale: 8333
    color_temp_state_topic: home/atomic_sun_v2/color/state
    color_temp_command_topic: home/atomic_sun_v2/color/set
|

That is what _fillBufferAndProcessTemplates() returns

The 100% is espaced in the template "100%%". In case that matters somehow

length=977

|lure:</label> <select class="form-control" name="restore_level" id="restore_level"> <option value="0" >Do not turn on</option> <option value="1"  selected>Restore last brightness level</option> </select> </div> <div class="form-group"> <label for="max_temperature">Max. Temperature (°C):</label> <input type="text" class="form-control" name="max_temperature" id="max_temperature" value="75" placeholder="75"> </div> <div class="form-group"> <label for="temp_check_int">Temperature Check Interval (seconds):</label> <input type="text" class="form-control" name="temp_check_int" id="temp_check_int" value="30" placeholder="30"> </div> <div class="form-group"> <button type="submit" class="btn btn-primary">Save Changes...</button> </div> </form> <div class="form-group"> <textarea id="hass_yaml" rows="15" style="width:100%">light:
  - name: KFCAC52DB
    platform: mqtt
    availability_topic: home/atomic_sun_v2/status
    payload_available: 1
    payload_not_available: 0
   |

This is what it is supposed to return:

length=1064

|lure:</label> <select class="form-control" name="restore_level" id="restore_level"> <option value="0" >Do not turn on</option> <option value="1"  selected>Restore last brightness level</option> </select> </div> <div class="form-group"> <label for="max_temperature">Max. Temperature (°C):</label> <input type="text" class="form-control" name="max_temperature" id="max_temperature" value="75" placeholder="75"> </div> <div class="form-group"> <label for="temp_check_int">Temperature Check Interval (seconds):</label> <input type="text" class="form-control" name="temp_check_int" id="temp_check_int" value="30" placeholder="30"> </div> <div class="form-group"> <button type="submit" class="btn btn-primary">Save Changes...</button> </div> </form> <div class="form-group"> <textarea id="hass_yaml" rows="15" style="width:100%">light:
  - name: KFCAC52DB
    platform: mqtt
    availability_topic: home/atomic_sun_v2/status
    payload_available: 1
    payload_not_available: 0
    state_topic: home/atomic_sun_v2/state
    command_topic: home/atomic_sun_v2/set
    pa|

The next call returns the rest of the data and template

length=446

|yload_on: 1
    payload_off: 0
    brightness_state_topic: home/atomic_sun_v2/brightness/state
    brightness_command_topic: home/atomic_sun_v2/brightness/set
    brightness_scale: 8333
    color_temp_state_topic: home/atomic_sun_v2/color/state
    color_temp_command_topic: home/atomic_sun_v2/color/set
</textarea> </div> <div class="row"> <div class="col footer">&nbsp;</div> </div> </div> <script src="js/fc882422.js"></script> </body> </html>|

@philbowles
Copy link
Contributor

It would really help to see the actual code you are using to send this especially how the data is declared / defined ...is it just in one big "String" etc?

@sascha432
Copy link
Contributor Author

I wrote a small test to reproduce this bug

output

[44]|01234567890test_abcdefghijklmnopqrstuvwxyz01|
[40]|MNOPQRSTUVWXYZ0123456789_test01234567890|
---
[84]|01234567890test_abcdefghijklmnopqrstuvwxyz01MNOPQRSTUVWXYZ0123456789_test01234567890|

expected


[64]|01234567890test_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKL|
[40]|MNOPQRSTUVWXYZ0123456789_test01234567890|
---
[104]|01234567890test_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_test01234567890|

#include "ESPAsyncWebServer.h"

String callback_func(const String &str) {
    if (str == "REPLACE_ME") {
        return "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    }
    return String();
}

class TestAsyncAbstractResponse : public AsyncAbstractResponse {
public:
    TestAsyncAbstractResponse(AwsTemplateProcessor callback) : AsyncAbstractResponse(callback) {
        _template = "01234567890test_%REPLACE_ME%_test01234567890";
        _code = 200;
    }
    virtual size_t _fillBuffer(uint8_t *buf, size_t maxLen) {
        maxLen = std::min(_template.length(), maxLen);
        memcpy(buf, _template.c_str(), maxLen);
        _template.remove(0, maxLen);
        return maxLen;
    }
private:
    String _template;
};

void test_abstract_response() {
    TestAsyncAbstractResponse *resp = new TestAsyncAbstractResponse(callback_func);
    char *out = (char *)malloc(1024);
    char *ptr = out;
    uint8_t buf[64];
    size_t len;

    Serial.println();
    while((len = resp->_fillBufferAndProcessTemplates(buf, sizeof(buf)))) {
        Serial.printf("[%u]|%*.*s|\n", len, len, len, buf);
        memcpy(ptr, buf, len);
        ptr += len;
    }
    *ptr = 0;
    Serial.println("---");
    Serial.printf("[%u]|%s|\n", strlen(out), out);
    free(out);
}

sascha432 added a commit to sascha432/ESPAsyncWebServer that referenced this issue Aug 28, 2019
me-no-dev pushed a commit that referenced this issue Oct 2, 2019
* Added method to access clients of AsyncWebSocket

* #571

* Conflict with crypto library
@stale
Copy link

stale bot commented Oct 21, 2019

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 21, 2019
@fleixi
Copy link

fleixi commented Oct 22, 2019

I still have the same problem if i try to upload some small file via the AsyncWebServer.

server.on("/upload", HTTP_POST, [](AsyncWebServerRequest * request) {}, [](AsyncWebServerRequest * request, const String & filename, size_t index, uint8_t *data, size_t len, bool final) { if (filename == "index.html" || filename == "init.html" || filename == "styles.css" || filename == "scripts.css") { if (!index) { Serial.printf("Hochladen von: %s\n", filename.c_str()); } File file = SPIFFS.open("/" + filename, "w"); for (size_t i = 0; i < len; i++) { file.write(data[i]); } if (final) { file.close(); Serial.println("Hochladen erfolgreich"); request->redirect("/"); } }

Here my files with problems
styles.css.txt #The file i upload
styles.css.truncat.txt #The file i get on the esp

Uploading a new firmware using Updater.h working fine. There only problems with small textfiles for me.

This is my htmlcode for uploading the files:

form class="upload_form" action="upload" method="post" enctype="multipart/form-data"> <label class="upload">Wählen Sie die hochzuladenden Dateien von Ihrem PC aus: <br> <input class="upload" name="datei" type="file"> <br> </label> <button>Hochladen</button> </form>

Im using the last master with the workaround added but without success

@stale
Copy link

stale bot commented Oct 22, 2019

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the stale label Oct 22, 2019
@stale
Copy link

stale bot commented Dec 21, 2019

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 21, 2019
@me21
Copy link
Contributor

me21 commented Dec 24, 2019

I think it's still relevant, no?

@stale
Copy link

stale bot commented Dec 24, 2019

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the stale label Dec 24, 2019
@sascha432
Copy link
Contributor Author

Looks like my "fix" was merged into the main branch. I guess I had that in my pull request for something else. So far this is working for me, I did not have any issue with templates anymore.

403752a#diff-2bfef7483934cf6c10c5c03a8553d5b0

@stale
Copy link

stale bot commented Feb 23, 2020

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 23, 2020
@stale
Copy link

stale bot commented Mar 8, 2020

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Mar 8, 2020
DanielKnoop pushed a commit to DanielKnoop/ESPAsyncWebServer that referenced this issue Jun 17, 2022
* Added method to access clients of AsyncWebSocket

* me-no-dev#571

* Conflict with crypto library
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants