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

special chars should not be allowed in id #207

Closed
berndbischl opened this issue Feb 6, 2019 · 9 comments
Closed

special chars should not be allowed in id #207

berndbischl opened this issue Feb 6, 2019 · 9 comments

Comments

@berndbischl
Copy link
Sponsor Member

@mllg brought this up here
#206

we should check that a user CANNOT enter anything weird in an id

@mb706
Copy link
Contributor

mb706 commented Feb 6, 2019

Checking this in all exported functions on all things that could eventually turn into a set_id seems like a high price to pay for a regex call.

@mllg
Copy link
Sponsor Member

mllg commented Feb 6, 2019

The regex is not the point. If you do an assertion (in the super class?), you can be sure that you can use the ids in data.table columns, file names and formulas.

@berndbischl
Copy link
Sponsor Member Author

you do an assertion (in the super class?), you can be sure that you can use the ids in data.table columns, file names and formulas.

thats why i created this issue....
whats the best way to guard against this? checkmate function?

@mllg
Copy link
Sponsor Member

mllg commented Feb 6, 2019

assert_string(id, pattern = "^[[:alpha:]]+[[:alnum:]_.]*$") should work...

But what if the user specifies an id with a "." in it? Your way of pasting and splitting ids does not seem to work in this case?

@berndbischl
Copy link
Sponsor Member Author

assert_string(id, pattern = "^[[:alpha:]]+[[:alnum:]_.]*$") should work...

shouldnt something similar like this even be a helper in cm? seems pretty common case...?

But what if the user specifies an id with a "." in it? Your way of pasting and splitting ids does not seem to work in this case?

i thought it did? the operations are this:
a) paste the setid to the param id, with a dot --> works if setid or paramid has a dot
b) replace setid in setid.param with emptystring --> works
c) split off left most dot --> doesnt work if setid contains a dot...
hmm. c) is faulty?

@berndbischl
Copy link
Sponsor Member Author

i think i could change c), because that is only used when the setids and paramids are already known.
so i could split wrt to them? hmm maybe a bit cumbersome

@berndbischl
Copy link
Sponsor Member Author

i would be nicer if the separating dot would be "clearer" / protected.
but i cannot disallow dots in parameter names....
i made the suggestion to use "#" as a separating char, or ":", you didnt wnat this (i guess with reason) as we now create invalid ids...

@berndbischl
Copy link
Sponsor Member Author

But what if the user specifies an id with a "." in

has its own issue now:
#212

@berndbischl
Copy link
Sponsor Member Author

we are done here, i added helper assert_id and use this in param ids and paramset ids

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

3 participants