Fix regex and add more command support to config parser#219
Conversation
|
Cool. This looks like a real implementation. The original code was more like a proof-of-concept... |
|
I'l test over the weekend.. haven't touched code since my uni days 30 years ago! These AI tools are getting very helpful.. |
|
I wanted to mention a point of caution: The JavaScript and HTML end up in the flash of the switch and will be transferred from the switch to the Browser when the page loads. Unlike for C-code which is turned into machine-code bytes without any of the comments and without taking lengths of variables into account in the device's flash, this will include all comment-texts. Variables are stored/transferred with every single byte of their name. The flash memory of most devices is probably large enough not to be worried, but transferring the data will result in longer times for the browser to display the page, because the IP-stack of the devices is very slow. Roughly every frame of up to 2500 Bytes takes 20ms to transfer. Since the Browser sends 6-8 requests in parallel, there can be also timeouts of requests because uIP handles only one request at a time. In that case you see the page being built up by the browser in a couple of seconds, only. We can probably improve that by using some kind of html/JS compression, but it will add a layer of complexity. On the other hand, we should also think about translated pages, and there, there will also be some kind of processing of the pages, I imagine. |
I guess compressing / minifying html/.js code at compile time shouldn't be a problem at all, there are plenty of tools. With other words: In any case, we should not omit any comments or better readable code just because we "fear" longer loading times. |
|
I did some testing, and so far everything works like a charm. I will need some more time for testing, though, because I see that the changes are across many files. I also only casually looked at the code. |
|
Don't know if I find some time to test (have no knowledge in html/js code, so reading the diff is useless in my case). Side question, related to github: What is the easiest/fastest way to get the code of this PR? Just clone the mcaptur repo? For such small repo that's not a problem, but in case of huge repos that seems not very efficient.... |
|
Im Building a new ui.. give me sometime as had to go out and need to fix some kinks.. its much more responsive now.. but will need some ppl to test |
As usual, there is solution in git ;-) Note that you need to configure the upstream remote repository. You basically get a branch for the PR in your local git repository. |
Hi Marc, can you separate the different aspects of the code you are going to write into different PRs? This PR here is about fixing the configuration storage in flash. And maybe a different one for the UI? This makes testing easier and also prevents feature-creep, i.e. if you have an omnibus for all your work, people will also start to ask more and more from you. Alternatively, you will put more and more in, just because it fits. But if you have something specific this is much less of a problem. |
|
built a new ui.. much faster than previous one.. i think everything mostly works now I know its a diff approach but give it a spin and it is much less taxing on the embeded cpu.. |
Multiple related bugs in the Save-to-Flash path:
1. Multipart upload was missing the required filename argument,
causing the backend parser to fail. Added 'config.txt' to the
form.append call. This was likely the primary reason Save-to-Flash
was unreliable.
2. Web UI was not pausing its polling interval during flash write,
causing CPU contention and intermittent crashes. Added isSaving
lock and clearInterval before sendConfig.
3. conf_cmds whitelist was incomplete. Added: syslog, passwd, pvid,
ingress, port name, lag, laghash, isolate, stp, igmp, mtu, bw,
vlan N mgmt, vlan N d.
4. VLAN regex blocked named VLANs. New pattern allows optional name
(starts with letter, matching CLI parser semantics).
5. vlan N d (delete) was not persisted. parseConf now removes the
matching vlan N ... entry from configuration[] when seeing a
delete command, without storing the delete itself. Result: the
saved config describes the end state.
6. configuration[] was not cleared between flashSave invocations,
leading to stale entries from prior interactions.
7. conf_overwrite boundary fix: 'pvid 1' no longer matches 'pvid 10'
etc. Added trailing space in startsWith check.
8. All conf_cmds patterns now anchored with ^...$ for full-line
match. parseConf normalizes whitespace before testing.
9. Port range widened to \d{1,2} so ports 11+ are accepted.
Structural fixes (1, 2, 6, 7, 8) ported from mcaptur's closed PR #219;
remaining fixes (3, 4, 5, 9) and overall regex strategy are new.
Multiple related bugs in the Save-to-Flash path:
1. Multipart upload was missing the required filename argument,
causing the backend parser to fail. Added 'config.txt' to the
form.append call. This was likely the primary reason Save-to-Flash
was unreliable.
2. Web UI was not pausing its polling interval during flash write,
causing CPU contention and intermittent crashes. Added isSaving
lock and clearInterval before sendConfig.
3. conf_cmds whitelist was incomplete. Added: syslog, passwd, pvid,
ingress, port name, lag, laghash, isolate, stp, igmp, mtu, bw,
vlan N mgmt, vlan N d.
4. VLAN regex blocked named VLANs. New pattern allows optional name
(starts with letter, matching CLI parser semantics).
5. vlan N d (delete) was not persisted. parseConf now removes the
matching vlan N ... entry from configuration[] when seeing a
delete command, without storing the delete itself. Result: the
saved config describes the end state.
6. configuration[] was not cleared between flashSave invocations,
leading to stale entries from prior interactions.
7. conf_overwrite boundary fix: 'pvid 1' no longer matches 'pvid 10'
etc. Added trailing space in startsWith check.
8. All conf_cmds patterns now anchored with ^...$ for full-line
match. parseConf normalizes whitespace before testing.
9. Port range widened to \d{1,2} so ports 11+ are accepted.
Structural fixes (1, 2, 6, 7, 8) ported from mcaptur's closed PR logicog#219;
remaining fixes (3, 4, 5, 9) and overall regex strategy are new.
3 participants
Changes are rarely saved! this shoudl fix but needs more testing