-
Notifications
You must be signed in to change notification settings - Fork 1
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
Consider regular expression for --dataset DATASETID #66
Comments
I agree this is useful and the cdasws supports regex for many "dataset search" query parameters. But implementing it is difficult if you defend against ReDoS. I had to protect the cdasws from ReDoS years ago. It never caused a problem until after a recent upgrade when many (simple) requests involving client-supplied regex values were rejected as being "too complex". I never understood what happened but it hasn't happened again (even though the same regex request is sent 100s/1000s of times/day). Sometimes I wish I never implemented regex support nearly 20 years ago. |
I didn't look closely at the project for this issue. I thought you were suggesting adding regex support to the hapi specification. ReDoS is less of a concern for the verifier. So, ignore my previous comment. |
That's good to know, Bernie. I was thinking of it in terms of running it locally, but I can see that with the server mode this would be a volnerability. So maybe a constrained set of regular expressions, or maybe wildcards (--exclude=BAR*)? (The validator has spent over an hour and it is still going through BAR*... I hope this isn't causing any problems for you, Bernie.) |
I see the --help option shows that "^regex" is supported, so it's really more of a documentation issue. I'd like to try:
but this doesn't seem to work. Also, how would I use a carot in the regex, is it just:
|
Not sure what the issue is. These work:
To debug, put I'll add some examples to the docs in the future. Regarding escaping, the code is very simple: https://github.com/hapi-server/verifier-nodejs/blob/master/tests.js#L1399. So I'd experiment in Chrome Debugger, for example, I'd write a script for more complex use cases that generates a sequence of |
Should this mean test any dataset that starts with D:
I was expecting it to do the D's. Is this not right? |
I would think so. Are you using the latest version? Try But given you saw the regex option in the help, it seems you are using a version with the regex feature. If I do the `console.log(datasets); process.exit()`` in place of line 1420 and your command, I see
|
I should have started by pulling the latest code. This is working for me now. Do you know if I can do --dataset='^^BAR.*' to exclude all the BAR ones? (It doesn't seem to work for me.) |
Probably. I'd try the hints at https://stackoverflow.com/questions/1538512/how-can-i-invert-a-regular-expression-in-javascript and test in https://regex101.com/ (make sure to select Javascript on left). Perhaps |
It would be useful for me to be able to run suites of tests on parts of a server, maybe by specifying a regular expression for the --dataset argument. For example, testing the new CDAWeb HAPI server there are many Barrel mission CDF files, which are all very similar and it would be nice to exclude them from the test (with ^(?!BAR).+ for example).
The text was updated successfully, but these errors were encountered: