Skip to content

replace encode calls with encodeURIComponent #4

Closed

Description

@donpark

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.

Activity

  1. scripting commented on Jul 13, 2016

    @scripting
    Owner

    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
    #4, or mute the thread
    https://github.com/notifications/unsubscribe/ABm9O5HEVB9OdtTl5WIuIh5PbgfU-wiQks5qVBAdgaJpZM4JK4GQ
    .

  2. donpark commented on Jul 13, 2016

    @donpark
    Author

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

  3. donpark commented on Jul 19, 2016

    @donpark
    Author

    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.

  4. donpark commented on Jul 19, 2016

    @donpark
    Author

    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.

  5. donpark commented on Jul 19, 2016

    @donpark
    Author

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

    screenshot_181

  6. donpark commented on Jul 20, 2016

    @donpark
    Author

    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 d49420f

  7. scripting commented on Jul 20, 2016

    @scripting
    Owner

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

  8. donpark commented on Jul 20, 2016

    @donpark
    Author

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

  9. scripting commented on Jul 20, 2016

    @scripting
    Owner

    "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
    d49420f

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions