Skip to content

Actual ICU errors are discarded #65

Closed
@ezzatron

Description

@ezzatron

I was trying to use this lib to implement SASLprep, in order to support Unicode usernames. I get true for prep.isNative(), and strings can be prepared so long as ICU does not throw an error. I also disabled JS fallbacks via prep.disableJsFallbacks().

When there is a genuine error, such as a U_STRINGPREP_UNASSIGNED_ERROR, the original error is discarded, and libicu unavailable is thrown instead. The relevant source code reads:

    try {
        if (this.stringPrep) {
            return this.stringPrep.prepare(this.value)
        }
    } catch (e) {}
    if (false === this.useJsFallbacks) {
        throw new Error(this.LIBICU_NOT_AVAILABLE)
    }
    return this.jsFallback()

It would be much more useful to have the original exception. Is there some reason why the original exception is not thrown?

In addition, I think there is a much bigger problem. If you're using stringprep for something security related, the side-effects of the above code are even worse when you leave JS fallbacks enabled. In that case, even when ICU bindings are working instead of throwing the error, the code silently falls back to a potentially insecure JS substitute, and a consumer of node-stringprep might never notice this happening.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions