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.

Sign up for GitHub

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

Jump to bottom

Pre parsing validation #186

Open
acorbel opened this issue Sep 19, 2016 · 4 comments · May be fixed by #219
Open

Pre parsing validation #186

acorbel opened this issue Sep 19, 2016 · 4 comments · May be fixed by #219

Comments

@acorbel
Copy link

acorbel commented Sep 19, 2016 •
edited
Loading

I've read the following issue: #166 and I'm wondering how we can enforce type validation (using is module or node-validator).

If you submit an object to a string field, it is viewed as valid ("[Object object]"). Of course, I don't think it is the desired behaviour. What do you think about it?

Or maybe providing a generic field and we add this kind of validation:

exports.typeValidator = function (type, message) {
    if(!is.fn(is[type])) {
        throw new TypeError('this type is not defined');
    } 

    var msg = message || 'Please enter a valid value';
    return function (form, field, callback) {
        if (is[type](field.data)) {
            callback();
        } else {
            callback(msg);
        }
    };
};
@ljharb
Copy link
Collaborator

ljharb commented Sep 19, 2016

Forms are used to validate values sent over the wire - how can you submit an object value to it?

I'm definitely on board with adding is-based validation, but I'm not sure I understand where it would be valuable.

In other words, the [object Object] conversion likely happens in the browser, long before forms comes into play.

@acorbel
Copy link
Author

acorbel commented Sep 19, 2016

Ok. I understand. I explain my use case.
It's a simple REST API used with an SPA (AngularJS). As usual, I don't trust frontend submission and I use this lib to parse the JSON input and validate it. So, I need to care about the field type.
Perhaps it is not the main use case but I've found this lib very useful, I'm trying to find some solution to make it more powerful ;)

@ljharb
Copy link
Collaborator

ljharb commented Sep 19, 2016

I'm all about adding explicit is.string and such tests - i don't like is[type] though.

@voxpelli
Copy link
Contributor

If I have a field foo then some form data parsers will parse a posted field foo[bar]=abc as { bar: 'abc' }. That's eg. the default in PHP.

And foo=abc&foo=xyz is by some parsers parsed as ['abc', 'xyz'], so it is possible to get objects and arrays out of fields that weren't intended to be that, all depending on ones parser – and since one will often be using a parser outside of forms itself, it all depends on how one configures it.

And now after writing this I realize that you are the maintainer of one of the major such parsing libraries @ljharb: https://github.com/ljharb/qs 😅

Would it maybe be better to ignore the data when parsing, rather than type casting it?

forms/lib/fields.js

Lines 24 to 26 in 3fb0edc

if (typeof raw_data !== 'undefined' && raw_data !== null) {
return String(raw_data);
}

forms/lib/fields.js

Lines 114 to 119 in 3fb0edc

f.parse = function (raw_data) {
if (raw_data === null || raw_data === '') {
return NaN;
}
return Number(raw_data);
};

Or maybe even throw an exception if the data is set, but is set to something else than a string in eg. those two cases?

voxpelli added a commit to voxpelli/forms that referenced this issue Jan 19, 2021
@voxpelli
Don't type cast non-sensible values
e792ed0 
Fixes caolan#186
voxpelli added a commit to voxpelli/forms that referenced this issue Jan 19, 2021
@voxpelli
Don't type cast non-sensible values
9cd40c5 
Fixes caolan#186
@voxpelli voxpelli linked a pull request Jan 19, 2021 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't type cast non-sensible values voxpelli/forms
3 participants
@voxpelli @ljharb @acorbel