Bug: Runtime Type Error in Cache-Parser Where Header Token Contains no ,

[nagilson]

What happened?

This code: https://github.com/arthurfiorette/tinylibs/blob/main/packages/cache-parser/src/util.ts#L18 , which this library seems to depend on, is failing at runtime for us in a rare number of cases.

(I was very relieved to see you manage both libraries, so thank you for the work you do :). )

My very quick glance is the problem is the code seems to rely on the fact that , is in the header of the string that is passed to it. I dont think that assumption can always hold true, so sometimes the token ends up being null or a type other than string, so when .split is called on it, there becomes an uncaught runtime error. The split on line 22 is the culprit.

I don't know much more of the context of the code here and there might be some other reason, but that is my suspicion.
Example is dotnet/vscode-dotnet-runtime#1734.
I don't have a repro, this info is garnered from customers.
 
It is causing some issues for us over in VS Code land and it is one of our highest causes of failure, causing roughly .01% out of several million customers to fail, with a high report rate on mac os sonoma silicon m1.

axios-cache-interceptor version

^1.0.1

Node / Browser Version

16

Axios Version

^1.3.4

What storage is being used

Memory Storage

Relevant debugging log output

Failed: n[r].split is not a function. Aborting. Stack: TypeError: n[r].split is not a function
    at /usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/esm|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\cache-parser\dist\index.mjs:1:425
    at parse (/usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/esm|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\cache-parser\dist\index.mjs:1:327)
    at Function.headerInterpreter (/usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/dynamic|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\axios-cache-interceptor\dist\index.cjs:2:1324)
    at n (/usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/dynamic|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\axios-cache-interceptor\dist\index.cjs:2:5445)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at Pt.request (/usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/dynamic|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\axios\dist\node\axios.cjs:3872:7)
    at Pt.request (/usr/local/google/home/USER/.vscode-insiders/extensions/ms-dotnettools.vscode-dotnet-runtime-2.0.5/javascript/dynamic|c:\Users\USER\vscode-dotnet-runtime\vscode-dotnet-runtime-library\node_modules\axios\dist\node\axios.cjs:3877:41)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

[nagilson]

cc @baronfel

[arthurfiorette]

Hey @nagilson, can you setup a small reproducible example or paste here a Cache-Control header exemple value that throws the above error?

[nagilson]

Thank you so much for your very fast reply.

May you help me gather that information for you? The header seems to be generated from Axios-Cache-Interceptor, so I'm not sure where it's generated, or what it is, to be honest. Is there something we could type in the developer browser console to access that information?

The web request this is for is one for: https://dot.net/v1/dotnet-install.sh
with these options: https://github.com/nagilson/vscode-dotnet-runtime/blob/a77568a7b983d3565bbb571d31ec550442c953b5/vscode-dotnet-runtime-library/src/Utils/WebRequestWorker.ts#L168.

It does not always happen, in fact I have not been able to get it to happen. So giving a clear repro is also challenging. The real repo is downloading VS Code and running .NET, and having one of the very small weird scenarios that seems to cause this web request to fail.

[arthurfiorette]

Source:

const tokens = headerStr.toLowerCase().replace(/\s+/g, '').split(',');

for (const token of tokens) {
  const split = token.split('=', 2) as [string, string];
  headers[split[0]] = split[1] ?? true;
}

Transpiled:

var a = {},
  r = e.toLowerCase().replace(/\s+/g, '').split(',');

for (var t in r) {
  var s,
    n = r[t].split('=', 2);
  a[n[0]] = null == (s = n[1]) || s;
}

r is the result of e.toLowerCase().replace(/\s+/g, '').split(',') which shouldn't be something else than an array, then it gets iterated in a for (var t in r) loop.

Normally, r[t] cannot be undefined:

Am I missing something or this seems to be a bug in the js runtime?

[nagilson]

@arthurfiorette The more I am looking at the code, I am not seeing what value would cause this to happen yet either. One sec...

[arthurfiorette]

Maybe String.prototype.split was modified by another library?

[nagilson]

Hm, that's a good theory. I checked and it looks like it's coming from lib.es2015.symbol.wellknown.ts, so unfortunately probably not.

What I am suspicious about is that my minified code is identical to yours, and despite the fact that the callstack aligns with the .split in my file, the error is on n[r].split, but the split on the character line is n = r[t].split(). I don't know why that'd be the case, maybe its just the randomly generated chars in the error for an arbitrary index operation. If you have an idea, or there is more I can do to get the header string for you based on the above, please let me know.

Based on the Javascript spec, this should not be possible. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split#:~:text=Note%3A%20%22%22.split(%22%22)%20is%20therefore%20the%20only%20way%20to%20produce%20an%20empty%20array%20when%20a%20string%20is%20passed%20as%20separator%20and%20limit%20is%20not%200. Split would never return an empty array, and you loop for token in tokens anyway. Token would have to be a non string type, but I dont see in the spec how that is possible with string.

[nagilson]

I'm going to double check with our customer to see if their string split method has been modified. (Note this has happened more than a few times, so I would not resolve it to a bit flip...)

[nagilson]

Another thought: Typescript checks are only at compile time. Does any of your code edit variables at runtime in places it would not do so at compile time?
I have an idea to collect the header from the user. If that does not work, I guess we'll need to prioritize and see if we have time to try some obscure linux distros and repro it.

[arthurfiorette]

I actually have no clue on how to solve this issue... I build the project in my machine and the output seems to have changed, probably because the bundler have been updated since the latest cache-parser release.

I could try to release a new version with this different bundle and see if this error stops...

[arthurfiorette]

Does any of your code edit variables at runtime in places it would not do so at compile time?

Everything gets executed the same way it's written...

[arthurfiorette]

Hey @nagilson any updates?

[nagilson]

@arthurfiorette Apologies, I was on vacation. The thing I can confirm is that the string split method was not modified. I don't have any further insights at the moment. So I would appreciate you giving it a go with the bundler to see if that fixes the issue.

Theoretically there could also be some sort of 'guard' in place in the code even though we don't think its possible to encounter this situation, but that's not ideal coding of course. If it could be changed to collect more information for when it does happen for the next report we get, that'd be helpful (IDK if this has a log system.) Thanks for your continued investment in my issue.

[arthurfiorette]

Just released v1.2.5. Let's see if it fixes this weird problem :/

[arthurfiorette]

Hey @nagilson, axios-cache-interceptor v1.5.3 was released with cache-parser v1.2.5.

Let's wait some time to see if it fixes this weird issue :/

[nagilson]

Thanks! Ill get the update in our next release and see if it fixes it :)