scripting / nodeStorage

A simple storage system based on Twitter identity implemented in Node.js.
GNU General Public License v2.0
103 stars 9 forks source link

replace `encode` calls with `encodeURIComponent` #4

Closed donpark closed 8 years ago

donpark commented 8 years ago

While dockerizing nodeStorage, I ran into encode function undefined error. I did notice that there are several instances of encode function sprinkled around in both javascript and OPML files so there error was probably caused by trigger some area not typically covered.

Since encode function maps directly to encodeURIComponent, I think they can be replaced with direct calls except in one javascript file where encode is defined differently.

This is an enhancement only and not required for dockerizing.

scripting commented 8 years ago

It would be helpful to know where the bad call(s) are.

I did a quick search in the source code and didn't find an error such as the one you describe.

Next time it happens could you note the name of the file and the line number at which it occurred.

Dave

On Tue, Jul 12, 2016 at 6:07 PM, Don Park notifications@github.com wrote:

While dockerizing nodeStorage, I ran into encode function undefined error. I did notice that there are several instances of encode function sprinkled around in both javascript and OPML files so there error was probably caused by trigger some area not typically covered.

Since encode function maps directly to encodeURIComponent, I think they can be replaced with direct calls except in one javascript file where encode is defined differently.

This is an enhancement only and not required for dockerizing.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/scripting/nodeStorage/issues/4, or mute the thread https://github.com/notifications/unsubscribe/ABm9O5HEVB9OdtTl5WIuIh5PbgfU-wiQks5qVBAdgaJpZM4JK4GQ .

donpark commented 8 years ago

Will do. I was thrashing around trying to get things working so didn't capture the details.

donpark commented 8 years ago

Finally had time to resolve Twitter signin problem that appeared after initially getting the docker working. Turned out to be a typo on my part. At least I can now see the encode problem again.

Here is the log output:

3:48:41 AM 613.38MB GET 1999.docuverse.com:1999 /chatlogindex http://1999.docuverse.com:1999/ ::ffff:192.168.99.1
3:48:51 AM 611.81MB GET 1999.docuverse.com:1999 /connect http://1999.docuverse.com:1999/ ::ffff:192.168.99.1
3:48:53 AM 611.44MB GET 1999.docuverse.com:1999 /callbackfromtwitter  ::ffff:192.168.99.1
/app/storage.js:2555
                      var url = parsedUrl.query.redirectUrl + "?oauth_token=" + encode (accessToken) + "&oauth_token_secret=" + encode (accessTokenSecret) + "&user_id=" + encode (results.user_id) + "&screen_name=" + encode (results.screen_name);
                                                                                ^

TypeError: encode is not a function
    at /app/storage.js:2555:70
    at /app/node_modules/node-twitter-api/twitter.js:48:4
    at /app/node_modules/oauth/lib/oauth.js:472:12
    at passBackControl (/app/node_modules/oauth/lib/oauth.js:390:11)
    at IncomingMessage.<anonymous> (/app/node_modules/oauth/lib/oauth.js:409:9)
    at emitNone (events.js:91:20)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:973:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickDomainCallback (internal/process/next_tick.js:122:9)

loadConfig: config == {
    "myPort": 1999,
    "websocketPort": 2000,
    "where": {
        "flUseLocalFilesystem": true,
        "privatePath": "/data/privateFiles/",
        "publicPath": "/data/publicFiles/"
    },
    "updates": {
        "enabled": false
    }
}

nodeStorage v0.95a running on port 1999, freemem = 612.96MB, urlWhitelist == undefined

openAllUserChatlogs: theList == []
startup: websockets port is 2000
3:48:59 AM 610.78MB GET 1999.docuverse.com:1999 /callbackfromtwitter  ::ffff:192.168.99.1
twitter.getAccessToken: error == undefined

/app/storage.js line 255 is in switch handling code for case "/callbackfromtwitter":, specifically callback handler for twitter.getAccessToken call.

function encode on line 2184 should be visible in the callback function context but for some reason it's not. hmm.

donpark commented 8 years ago

Did you have similar encode function issue in /shortenurl case handler (line 2734)? I see that you have duplicate encode function there.

Strange part is /callbackfromtwitter handler is called by every running instance to complete sign-in to Twitter so it must be working for others yet it's not for me.

When I move encode function closer to just outside the handler switch block, I'm able to sign-in successfully which seems to indicate this might be a 'very large function' problem. If so, it's likely to popup again later. Only solution is, of course, to break it up into small functions.

donpark commented 8 years ago

Screenshot of nodeStorage running as a Docker container after moving the encode function down.

screenshot_181

donpark commented 8 years ago

I think issue #3 is caused by this bug, meaning this is a general issue.

Steps to replicate error condition:

  1. sign-off Twitter on 1999 home page
  2. sign-in to Twitter
  3. sign-in should fail with callbackFromTwitter URL

In my docker branch, I fixed the issue by replacing call to encode with call to encodeURIComponent in callbackFromTwitter handler as you can see in d49420f087a737237ae5249b2c634013e42f44ae

scripting commented 8 years ago

I added the workaround in v0.95b. Instead of moving "encode" -- I just call encodeURIComponent in the place that was having the problem.

donpark commented 8 years ago

lol. that's exactly the change I made in d49420f087a737237ae5249b2c634013e42f44ae

scripting commented 8 years ago

"Great Minds Think Alike."

On Wed, Jul 20, 2016 at 9:57 AM, Don Park notifications@github.com wrote:

lol. that's exactly the change I made in d49420f https://github.com/scripting/nodeStorage/commit/d49420f087a737237ae5249b2c634013e42f44ae

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/scripting/nodeStorage/issues/4#issuecomment-233956975, or mute the thread https://github.com/notifications/unsubscribe-auth/ABm9O1eVZqe0KLz6ulRGEZK9Zsai3izZks5qXiljgaJpZM4JK4GQ .