Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

Invalid JSON output for list items, possible too many items? #10

Closed
Mange opened this issue May 28, 2018 · 1 comment
Closed

Invalid JSON output for list items, possible too many items? #10

Mange opened this issue May 28, 2018 · 1 comment
Labels

Comments

@Mange
Copy link

Mange commented May 28, 2018

I get invalid JSON when I list all items in my vault.

Here are my test cases:

bw list items | python -mjson.tool
bw --response list items | python -mjson.tool

When doing --response and manually looking at the output, there's a lack of closing } to one of the items and then the output is cut off (no closing symbols for the parent elements).

Expecting ',' delimiter: line 1 column 65488 (char 65487

If I remove the --response part then I get an "unterminated string" error.

Unterminated string starting at: line 1 column 65465 (char 65464)

Hint: Both are suspiciously close to 2¹⁶ = 65536. 🤔

Hint: With the envelope from --response, the item that is cut off is earlier in the output.

Hint: If I search for any single normal letter like a or e I still get the error. If I add another letter so I would get fewer results, then everything works.

Hint: This is not a bug in Python; I'm just using Python as a simple test case. I get the same problem no matter which JSON library or program I use; even raw output can be verified to not being closed correctly.


I suspect that whatever code is writing to STDOUT is not buffering correctly, so it actually writes the maximum number of bytes that it allows and returns that number, but then the method does not write the next chunk of the buffer.

int written = fwrite(stdout, all_the_json, strlen(all_the_json));
// written is now close to 2**16, which could be < strlen(all_the_json).

// Something like this might solve it..?

int offset = 0;
int written = 0;
int len = strlen(all_the_json);

while (written < len) {
  written += fwrite(stdout, all_the_json + offset, len - offset);
}

As this codebase is written in JS/Node, then this kind of API would be hidden behind JS-specific abstractions. This Stack Overflow post seems to talk about a similar problem.

@kspearrin kspearrin added the bug label Jun 19, 2018
@kwkelly
Copy link
Contributor

kwkelly commented Jun 20, 2018

The problem is that bitwarden is calling process.exit() before it can finish writing to stdout. It's related to nodejs/node#784

The fix is to set the exit code and allow the program to exit naturally. See here. Specifically,

Calling process.exit() will force the process to exit as quickly as possible even if there are still asynchronous operations pending that have not yet completed fully, including I/O operations to process.stdout and process.stderr.

So what you need to do is

Rather than calling process.exit() directly, the code should set the process.exitCode and allow the process to exit naturally by avoiding scheduling any additional work for the event loop

For example, replacing the commented code in the snippet below with what's directly below fixes the issue

    private processResponse(response: Response) {
        if (!response.success) {
            if (process.env.BW_QUIET !== 'true') {
                if (process.env.BW_RESPONSE === 'true') {
                    writeLn(this.getJson(response), true);
                } else {
                    writeLn(chalk.redBright(response.message), true);
                }
            }
            //process.exit(1);
            process.exitCode = 1;
            return;
        }

        if (process.env.BW_RESPONSE === 'true') {
            writeLn(this.getJson(response), true);
        } else if (response.data != null) {
            let out: string = null;
            if (response.data.object === 'string') {
                const data = (response.data as StringResponse).data;
                if (data != null) {
                    out = data;
                }
            } else if (response.data.object === 'list') {
                out = this.getJson((response.data as ListResponse).data);
            } else if (response.data.object === 'template') {
                out = this.getJson((response.data as TemplateResponse).template);
            } else if (response.data.object === 'message') {
                out = this.getMessage(response);
            } else {
                out = this.getJson(response.data);
            }

            if (out != null && process.env.BW_QUIET !== 'true') {
                writeLn(out, true);
            }
        }
        //process.exit();
        process.exitCode = 0;

    }

As far as I can tell, there are only 3 instances of process.exit in the code, so this should be easy.

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

No branches or pull requests

3 participants