Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Can we call unbuffer automatically? #1465

Closed
philbooth opened this issue Sep 21, 2016 · 7 comments · Fixed by #1653
Closed

Can we call unbuffer automatically? #1465

philbooth opened this issue Sep 21, 2016 · 7 comments · Fixed by #1653
Assignees
Labels

Comments

@philbooth
Copy link
Contributor

philbooth commented Sep 21, 2016

Me and @shane-tomlinson were just looking into a problem that was caused by a failure to unbuffer some arguments before sending a request to the db server. Seems like it would be simple enough and less error prone to remove that burden and call unbuffer automatically, in a little wrapper around poolee or something.

@rfk
Copy link
Contributor

rfk commented Sep 21, 2016

+1 to simplifying this, but be careful - I seem to remember a bug with automagical buffering/unbuffering corrupting someone's device name because it happened to look like it was a hex string. @jrgm might remember more details.

@philbooth
Copy link
Contributor Author

I seem to remember a bug with automagical buffering/unbuffering corrupting someone's device name because it happened to look like it was a hex string.

Yep, I think it was the browser version that got corrupted. This is was why I was careful only to mention unbuffer in the description; calling Buffer.isBuffer should be fool-proof, whereas checking for hex characters definitely isn't.

@seanmonstar
Copy link
Contributor

I agree as well that auto buffering could lead to non-obvious bugs.

An approach I used in oauth and profile servers was instead to use the buf module to ensure values that needed to be were converted to buffers at the db level. So, for instance db.createRefreshToken(buf(uid), buf(token), Date.now()). It additionally has unbuf methods that are used when serializing to JSON. I felt the explicitness helped while I was developing those servers.

@vladikoff
Copy link
Contributor

@philbooth how do you want to proceed with this issue?

@philbooth
Copy link
Contributor Author

I still have plans for this, don't close it yet!

@seanmonstar
Copy link
Contributor

Thinking more on this, I feel stronger that maybe the best plan is to rip unbuffer out.

@rfk
Copy link
Contributor

rfk commented Jan 3, 2017

looking into a problem that was caused by a failure to unbuffer some arguments
before sending a request to the db server.

Also, I don't remember the details of this problem, but is it possible it would have been simpler to handle if auth-db-server did stricter validation and nicer error messages on its request payloads?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants