Both handlers turned the first character of the port word into an index
with no check at all. "mtu 0 100" computes '0' - '1' = 255, reads far past
the end of phys_to_log_port[9], and writes the size to whatever register
0x1250 plus that garbage points at. "pvid 0 2" walks the same path into
port_pvid_set(). lag and vlan already validate their port arguments; these
two just did not.
The port now has to be a single digit 1 to 9, which is exactly the range
the mapping table holds. A second digit or a stray letter falls to the
usage message.
"mtu show" also gained the return it was missing: after printing the table
it fell through, derived a port from the word "show" and printed the
garbage byte before the length check stopped it.
"vlan 5000 mgmt" parked the management interface on a VLAN the table
cannot hold, which quietly cuts management off. IDs above 4094 now fall
through to the usage message. 0 still switches the management VLAN off,
which is the documented way to disable it.
"pvid 1 5000" packed 5000 into the 12-bit PVID field and truncated on the
way, so the port ended up with a PVID nobody chose. 0 and anything above
4094 are refused now, matching what the VLAN table can hold. A failed
parse used to be dropped without a word; both cases print the usage line.
The handler threw away atoi_short()'s return and leant on the range test
alone. The range test cannot tell a failed parse from a small number, so
"mtu 1 99999" stopped at the partial 9999 and went through as a number
nobody typed. With the parse result checked, a failure is rejected with
the same message as an out-of-range value.
"mtu 1 0" was accepted. The chip takes it verbatim, the port reports 0x0000
back, and it stops passing frames: on a live 2.5G LAG member the LACPDU
receives moved by 4 in fifteen seconds against 21 on the sibling port, and
the partner went expired. Restoring the size brought both back.
Nothing shorter than a minimum Ethernet frame is a usable maximum, so the
range is now 64 to 16383. The upper end is unchanged and still comes from
the width of the field the value is written into.
This also covers most of #312 by accident: "mtu 1 abc" leaves the parse
result at 0 and now gets rejected on the bound rather than reaching the
register. It does not cover all of it. The handler still ignores what
atoi_short() returns, so "mtu 1 99999" stops on a partial 9999 and goes
through as a number nobody typed.
The GUI is not affected either way, it offers a fixed list of sizes.
vlan_create() and vlan_delete() wrote the ID straight into the table index
register. vlan_get() has refused anything >= 0xfff for a while, so reads were
guarded and writes were not: "vlan 4095 1 2" built an entry that no read path
can see, and IDs above that either miss the table or alias onto another VLAN.
Both writers now enforce the range vlan_get() already did, and parse_vlan()
rejects the same values with the usage message so the CLI says why. The check
sits after the "vlan 0 mgmt" branch, which legitimately takes 0 to switch the
management VLAN off.
Costs nothing in RAM: rtl837x_port.rel stays at DSEG 0, OSEG 5, and the image
still reports 10207 bytes of XDATA in use.
atoi_short() accumulated into a uint16_t without checking, so "vlan 65540"
wrapped to 4 and edited VLAN 4 instead of failing. atoi_byte() had the same
hole with "300" landing on 44. Both now refuse the digit that would push the
value past its type, before it lands.
The partial result is deliberately left alone rather than zeroed. Zeroing
would give the function one tidy rule, every failure leaves 0, but a caller
that ignores the return would then write that 0, and 0 is not a harmless
number everywhere. Set as a port MTU it stops the port taking frames: I put
0 on a live 2.5G LAG member and its LACPDU receives moved by 4 in fifteen
seconds against 21 on the sibling port, with the partner going expired.
Putting the size back recovered both. A wrong number does less damage than
that, and the real fix belongs in the callers that ignore the return anyway.
The test sits inside the loop rather than after it, so no wider accumulator
is needed and the parser stays off the internal RAM budget.
056a30a on the branch in #303 fixes atoi_byte a different way, by widening
the accumulator. Whichever lands first, the other hunk should go.
Move the MAC-derived default name out of main() into its own function and
call it after execute_config(), returning early when the config already
set a name - a configured switch then does no work for it at all.
The body deliberately has no local variables. Locals here - counters and
pointers alike - land in the 8051's internal-RAM overlay, and on an image
with LACP and STP both enabled that overlay is exhausted: a loop makes the
linker fail with "Could not get 8 consecutive bytes in internal RAM for
area OSEG". Moving the code into its own function does not help, since the
overlay is shared across the whole image, and hoisting the locals to xdata
does not either, because itohex() is inline and brings its own frame. This
only shows up in an integrated build; the branch on its own links fine.
Suggested-by: vDorst
The hostname and model fields were populated through element-existence
checks and empty-string fallbacks that the other fields on the page do
without. Assign them the same way.
Suggested-by: vDorst
Every switch came up as "RTLPlayground", so several of them on one
network were indistinguishable until someone configured a name. Append
the last three MAC octets (e.g. RTLPlayground-1ef924) - unique in
practice, still recognisable, and any "hostname ..." line in the startup
config overrides it as before.
Suggested-by: plaes
Typing "hostname" on its own cleared the name: with no argument the copy
loop never ran and the terminating NUL landed at index 0. Report the
current name instead, accept exactly one argument to set it, and reject
anything longer - a name with spaces tokenizes into several words, and
silently keeping only the first one is worse than an error. Walk the
buffer with a pointer, which the compiler codes better than indexing.
Suggested-by: vDorst
machine.machine_name is already reported in /information.json as hw_ver;
surface it read-only on the System Settings page so the exact build
target is visible in the UI, without adding a redundant JSON field.
Add a device hostname settable from the CLI (`hostname <text>`) and the
System Settings page. The value is sanitized on ingest to JSON-safe
printable ASCII (<=23 chars), stored in a shared __xdata buffer, seeded
to "RTLPlayground" at boot, persisted through the startup-config, and
reported in /information.json. It lives in the common header so other
modules can advertise it (LLDP uses it as the System Name TLV).
Root cause of the "browser login always bounces back with Wrong password!
while curl works": scan_header() read the session id from a fixed offset
into the Cookie header (p + 17), assuming "session=" is the first and only
cookie. Browsers keep stale cookies for a long time - e.g. an "admin" cookie
left over from this switch's VENDOR firmware - so the header can arrive as
"Cookie: admin=..; session=..", the fixed offset then points into the admin
value, authentication silently fails and every page bounces to login although
the password had been accepted. curl sends only "session=", which is why
command-line tests passed while a real browser (with that stale cookie) failed.
- scan_header(): scan the Cookie header for the actual "session=" key
(matched as "session" - is_word() requires a separator after the pattern
and '=' is on its list, the first value byte is not).
- is_word_x(): accept ';' as a terminating separator so the session value
also matches when it is not the last cookie in the header.
Verified on hardware end-to-end in a real browser WITH the stale "admin"
cookie present: login -> index.html, all pages and JSON endpoints work.
Real root cause of the "web login fails from a browser but works from curl":
the uIP httpd is built with UIP_CONF_MAX_CONNECTIONS = 1 and uses global
response state (outbuf/slen/session), i.e. it serves exactly one TCP connection
at a time and closes it after each response - but never advertises that via
the Connection header. A browser's HTTP/1.1 client therefore assumes the
connection may be persistent and can park it in its keep-alive pool for reuse;
a later request sent on that pooled connection hits one the server has already
closed, and a POST (unlike a GET) is never retried by the browser, so it can
be silently lost this way.
This adds "Connection: close" to every response so the browser does not pool
and reuse a connection the server is about to drop. On its own this did not
fully explain the reported login failures - the actual authentication bug is
fixed in the next commit (the Cookie header parsed at a fixed offset) - but it
is correct behaviour for a server that only ever handles one connection, and
removes one source of dropped requests.
Two hygiene fixes for the web UI, prompted by a login that appeared to fail
under privacy shields (Brave Shields / NoScript-family extensions):
- httpd: replace the partial "style-src 'self' 'unsafe-inline'" CSP with a
complete, first-party policy (default-src 'self'; script-src 'self'
'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:;
connect-src 'self'; form-action 'self'). Everything the UI needs is
same-origin; the explicit policy stops shields injecting their own
restrictive report-only probes (the noisy script-src-elem 'none' console
spam) and passes a strict-CSP audit. Verified: no CSP violations in-browser.
- login.html: add autocomplete="current-password" so password managers
recognise the field (they showed "unknown password" without it).
NOTE: these do NOT bypass a browser's LAN-device protection (NoScript "lan" /
Brave Shields), which strips the POST body of requests to a LAN address and is
why the login can fail in-browser while the same credentials work over curl.
That is a deliberate browser security feature; the user must allow the site in
their shields to log in. The backend password (default 1234) is unchanged and
correct.
d00f