search

nlf / blankie

a hapi CSP plugin
52 stars 20 forks source link

Graceful fail if scooter fails to get a userAgent #2

Closed joeybaker closed 10 years ago

joeybaker commented 10 years ago

I've got hapi doing some redirects and (apparently) that can cause the userAgent string to be empty. In that case, scooter returns undefined. This prevents blankie from crashing in that case.

Uncaught error: Cannot read property 'major' of undefined TypeError: Uncaught error: Cannot read property 'major' of undefined
    at internals.addHeaders (/node_modules/blankie/lib/index.js:167:37)
    at //node_modules/hapi/lib/handler.js:399:22
    at iterate (/node_modules/hapi/node_modules/items/lib/index.js:35:13)
    at done (/node_modules/hapi/node_modules/items/lib/index.js:27:25)
    at finalize (/node_modules/hapi/lib/handler.js:386:69)
    at onServerPreResponse (/index.js:51:32)
    at /hapi/lib/handler.js:399:22
    at iterate (/hapi/node_modules/items/lib/index.js:35:13)
    at Object.exports.serial (/node_modules/hapi/node_modules/items/lib/index.js:38:9)
    at /node_modules/hapi/lib/handler.js:382:15 14100617029
nlf commented 10 years ago

I'm having a hard time reproducing this one, as I haven't been able to get useragent to return undefined instead of an agent object. Is this still an issue for you? Can you log request.raw.req.headers for me so I can see what's actually causing the failure?

joeybaker commented 10 years ago

@nlf heh… this was long enough ago, that I don't remember all the details. I believe I was dealing with redirects at the time. Sorry for the lack of detail… either way, as you probably already decided, this is a harmless enough change, no?

nlf commented 10 years ago

The problem is I'm unable to write a test to cover it, so not only does code coverage fall below 100% but I also have no way to test for regressions.

joeybaker commented 10 years ago

hmmm… yes, totally fair. I'll think about a test. It should be possible if you mock useragent, no?