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

illegal access error from decodeURIComponent function. #25551

Closed
lmarkus opened this issue Jun 20, 2015 · 18 comments
Closed

illegal access error from decodeURIComponent function. #25551

lmarkus opened this issue Jun 20, 2015 · 18 comments

Comments

@lmarkus
Copy link

lmarkus commented Jun 20, 2015

Could you guys take a look at expressjs/express#2652

Might be a bug in the underlying V8 for Node 0.12.x

@MichaelHedman
Copy link

I am the original reporter of this error, and I agree that the error probably originates from within the V8 code.

This error prevents us from upgrading Node beyond version 0.11.14.

@jasnell
Copy link
Member

jasnell commented Jun 24, 2015

@domenic ... there's not a lot to go on here but can you take a look?

@domenic
Copy link

domenic commented Jun 24, 2015

First, of course, is the question of whether this still occurs on a modern V8 version, such as that included in io.js 2.x or io.js next-nightly.

@lmarkus
Copy link
Author

lmarkus commented Jun 24, 2015

Unfortunately, can't confirm that from my side. We haven't seen this in our dev environments, and can't deploy io.js in prod.

@lmarkus
Copy link
Author

lmarkus commented Jul 1, 2015

Bump.
Any luck looking into this?
This error continues to surface randomly

@jasnell
Copy link
Member

jasnell commented Jul 1, 2015

@lmarkus ... I unfortunately have not had the opportunity to look into it further yet. One thought, however: would you happen to be able to capture the input to the decodeURIComponent that is triggering the error. If the error really is occurring within the decodeURIComponent function in V8, and it's only happening in production, then it's possible that you're getting hit by inputs with invalid code sequences and V8 isn't handling it appropriately. Collecting a log of those inputs ought to allow you to construct a test case that can be used to diagnose the problem.

@lmarkus
Copy link
Author

lmarkus commented Jul 1, 2015

Fair enough. We'll try to capture that.

@MichaelHedman
Copy link

From what we've seen, when this error occurs ALL strings fail in decodeURIComponent, even empty strings. The problem is that it's really difficult to trigger/reproduce on Heroku (needs a lot of dyno restarts).

@lmarkus can probably reproduce this much easier if he's running in a private environment. Are you running docker btw?

@lmarkus
Copy link
Author

lmarkus commented Jul 7, 2015

No docker. Plain deploy + PM2

@jasnell
Copy link
Member

jasnell commented Jul 10, 2015

Ok, given this it's going to be difficult to diagnose. It's conceivably a problem in the older version of V8 we're using in which case you'd need to test against io.js to see if the issue has been resolved. If a specific issue in V8 can be identified, we can investigate backporting a fix but absent that, there's really not anything we can do here. I'm going to go ahead and close this issue for now, but we can reopen once there is new information and once it's clear that something needs to be fixed in node itself. Fair?

@jasnell jasnell closed this as completed Jul 10, 2015
@lmarkus
Copy link
Author

lmarkus commented Jul 10, 2015

That's going to be a tough one.
This is a production deployment at PayPal, so I definitely won't be able to get away with trying io.js.

We will be putting additional logging in place, to see if we can at least come up with a reproducible test case.

Now, if we are able to reproduce, could we realistically expect any action to be taken? (Assuming that this in fact points to V8).

@jasnell
Copy link
Member

jasnell commented Jul 10, 2015

Realistically it will depend on just how big of a fix is required.
Backporting patches and floating them in v0.12 or v0.10 is definitely still
possible but will depend entirely on what other changes to v8 are made. We
want this stuff to work so definitely not going to rule it out.
On Jul 9, 2015 10:54 PM, "Lenny Markus" notifications@github.com wrote:

That's going to be a tough one.
This is a production deployment at PayPal, so I definitely won't be able
to get away with trying io.js.

We will be putting additional logging in place, to see if we can at least
come up with a reproducible test case.

Now, if we are able to reproduce, could we realistically expect any action
to be taken? (Assuming that this in fact points to V8).


Reply to this email directly or view it on GitHub
#25551 (comment).

@dfoody
Copy link

dfoody commented Jul 13, 2015

We're seeing the same issue in production with 0.12.6 and 0.12.7
It definitely seems to be something related to process startup (it begins failing within 1m of the process starting - but, if the process gets past that point, it doesn't happen).

@dfoody
Copy link

dfoody commented Jul 13, 2015

So, a few more points of data about the issue: Once it happens decodeURIComponent then fails for all subsequent calls. We added the following code as we detect the issue (right before the process restarts):

        var decoded = "failed";
        var reason;

        try {  decoded = decodeURIComponent("ok"); }
        catch(ex) { reason = ex.message || ex; }

and decoded="failed", reason="illegal access" after this code.

@dfoody
Copy link

dfoody commented Jul 13, 2015

The other thing we've seen is that it's not always related to process startup. We've had one case where a process ran for 1hr 12m and handled over 5000 requests before it got into this error state.

@indutny
Copy link
Member

indutny commented Jul 14, 2015

I think I can probably fix it. Will need to have a test case to reproduce it, though.

@MichaelHedman
Copy link

Any progress here?

@dfoody
Copy link

dfoody commented Aug 3, 2015

We gave up and migrated to io.js (and I can confirm the problem does not occur on io.js).

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

No branches or pull requests

6 participants