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

protect system tables from (accidental) overwrite #912

Closed
1 task done
pq opened this issue Oct 14, 2019 · 4 comments
Closed
1 task done

protect system tables from (accidental) overwrite #912

pq opened this issue Oct 14, 2019 · 4 comments
Assignees

Comments

@pq
Copy link
Collaborator

pq commented Oct 14, 2019

follow-up from (#559), write-protect specific system tables:

...

/cc @tehn

@pq pq added this to the 2.3.0 milestone Oct 14, 2019
@pq pq self-assigned this Oct 14, 2019
@tehn tehn mentioned this issue Oct 14, 2019
6 tasks
@tehn
Copy link
Member

tehn commented Nov 22, 2019

@pq (i know you're away, so no rush!) wondering how to best approach this with other tables

ie, main user error that will happen is something like:

metro = 5

which plows the metro lib. but of course this gets reset on script reload, so maybe this isn't an issue beyond throwing a confusing error to the user.

however

norns = "rad"

will plow the script restart function.

probably not worth the effort to protect the depths of the table, but maybe worth protecting (with a nice error message) just the top level table for various things that may be accidentally overwritten.

what do you think?

@pq
Copy link
Collaborator Author

pq commented Dec 3, 2019

i'm all for write-protecting common top-level tables (at least). sub-tables should be just as easy and probably worth guarding a few too.

have we seen any issues w/ this approach? (sorry: can't recall!)

@tehn
Copy link
Member

tehn commented Dec 3, 2019

main issue with protecting some subtables is if they have elements that are supposed to be writable by design (i realize this is probably bad style)--- i'd need to go through and identify these.

perhaps we just make another function that read-only's the top-level table?

@pq
Copy link
Collaborator Author

pq commented Dec 3, 2019

main issue with protecting some subtables is if they have elements that are supposed to be writable by design (i realize this is probably bad style)--- i'd need to go through and identify these.

it looks like we anticipated this 🎉 and there's actually precedent w/ engine.name

engine = tab.readonly{table = require 'core/engine', except = {'name'}}

exception logic is here:

norns/lua/lib/tabutil.lua

Lines 206 to 210 in 96311a8

if (tab.contains(exceptions, k)) then
t[k] = v
else
error("'"..k.."', a read-only key, cannot be re-assigned.")
end

perhaps we just make another function that read-only's the top-level table?

sure. or if the list of exceptions is short, maybe just explicitly list them like we did for engine?

happy to help with either approach. 👍

@tehn tehn removed this from the 2.3.0 milestone Mar 5, 2020
@tehn tehn closed this as completed Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants