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
Special char in modules cause error when using require to load from storage #1583
Comments
I'm pretty sure that's actually a problem with how you're writing the file. The file doesn't contain the character If you created the string with https://www.espruino.com/File+Converter (or manually double-escaped the |
This is how modules are saved to storage
And this is the output when using the require statements
|
Yes, that's just what I said above then. I think it's reasonable to expect Espruino to error if it encounters char code 0 when trying to execute code. If you quote the string properly when saving to storage (so what's in storage actually matches what's in SBUS.js) then it'll work great. |
manually double-escaped the \ characters
I thought there is a difference between SBUS and a min version of SBUS that causes the trouble. |
That is strange - I thought in your example code each example broke. I'm not sure why the unminified version was working fine - unless potentially the format of the if statements meant that the code wasn't actually getting executed. ... or ... could you have just require'd it once before? That might have skipped the re-evaluation |
tested again, loads but as you said let's double-escaped the \ characters Affected files: DigoleBuf.js, DigoleHW.js, MQTT.js and SBUS.js |
There must be a difference when loading modules from folder (1) and from storage (2), tested with tinyMQTT. Adding the double-escaped the \ characters is working for (2) but does for (1). @olliephillips had to remove them again olliephillips/tinyMQTT@1fd8090#diff-8bbcea8cef04a070ebbbebb8cc491984 |
Maybe I'm misunderstanding... Can we go with a really simple example - returning a quote?
In a module that works. And you're saying that when you do:
But that if you do:
it works? But that if you put that extra slash:
into a module file then it doesn't? |
Simple examples might not suffice. And it may even be a module/use case specific issue. My findings which led me to remove the extra slash from tinyMQTT below:- With the double escaping, and loaded as normal (not from storage), tinyMQTT appeared to work fine. The problem manifested on the subscription end. It stopped receiving messages after 30 or so seconds (I was publishing at 1 second intervals and received 31 messages on the subscribing client and no more - consistently). As I recall, there was nothing to indicate a problem on Espruino. With double escaping removed, publishing was as before but client was able to receive subscribed messages indefinitely. The escaping was added so that the tinyMQTT module could be used from storage, something @MaBecker had been doing without problem as I understand. So really, what we can say, is that for tinyMQTT there is a an issue somewhere with double escaping, when the module is used normally, which is not an issue when used from storage. I'm inclined to believe the broker has a problem with the payload of the message (not tested), so I'm not sure if this an Espruino problem given the above. Just my thoughts. |
As far as I can tell this is not a bug or even close. I thought when this was closed I'd explained the issue, but I don't think I had properly, and changes were suggested for tinyMQTT that should never have gone in. It's literally just an issue with quotes:
Basically using templated (backtick) Strings to write modules to storage allows you to copy/paste them most of the time, but it's like any other String. If there's an escape character in it then the escape character gets interpreted as that String is parsed. The escape character never actually makes it into flash storage as it should do - you could check just by comparing the filesize of what's in Storage vs the actual module. The double escaping isn't some magic Espruino feature, it's just what you have to do if you want to include an escaped String inside another string... But if you're not putting a string in a string (eg when the module is a file) then you don't need it. @MaBecker did you submit any other PRs with double-escaping added? If so I think we should revert them. |
I totally agree! This is the way I suggest to replace '\x00' with String.fromCharCode(0)
No because it was not clear if this is the correct solution. So @gfwilliams decide by your own how to handle '\x00' used in this modules: DigoleBuf.js, DigoleHW.js, MQTT.js and SBUS.js |
I think the current solution (single quoted As with any string at all, you need to escape what goes inside it properly. If I tweaked those modules not to include the Maybe https://www.espruino.com/File+Converter should just be extended so that it'll produce a properly quoted templated String for you to copy and paste so you have something easier when you try to load into Storage? The real solution here would be to have a file loader in the Web IDE itself though. |
Sorry I can not see a way how to make this work if saving to storage. |
This doesn't actually have anything to do with modules, or storage. What you're saying is "I can't paste the contents of a file into a JS string without modification" - and that is true. There's no way around it. While it's obvious that you can't just copy/paste a JPEG file into a String, it's less obvious that you can't usually do it with a module either. In your example of
The length of the module in Storage won't be the same as the one you intended to upload (even with your tweaked module), because the module has been corrupted. With your suggested changes you can still make it work, but it's still corrupted in places. You're still trying to execute a module that has the char code 255 in it in a few places, which is far from ideal and may well break in the future. Let's go for a really simple example:
That's great. But what if you try and put that into a templated string and execute it?
Because
isn't valid JS. But we can't just tell people never to use the How do you fix it?All you need to do is use https://www.espruino.com/File+Converter to convert the file into a proper JavaScript string by escaping the bits that need escaping. I just added a new bit down the bottom where it'll create a templated String for you. All it does is replace |
The special char is \x00 like used many modules e.g. MQTT.js or SBUS.js.
Sample code test_case_module_sbus.js
The text was updated successfully, but these errors were encountered: