processing variables from external file #632

ricmrodrigues · 2020-07-03T15:27:43Z

this is part of an issue I've raised, maybe missing something but I tested and it seemed to work :)

Huachao · 2020-07-06T01:55:55Z

src/utils/httpRequestParser.ts

@@ -179,7 +180,10 @@ export class HttpRequestParser implements RequestParser {
                        const inputFilePath = groups[1];
                        const fileAbsolutePath = await resolveRequestBodyPath(inputFilePath);
                        if (fileAbsolutePath) {
-                            combinedStream.append(fs.createReadStream(fileAbsolutePath));
+                            const buffer = await fs.readFile(fileAbsolutePath);
+                            const fileContent = buffer.toString('utf8');


Here you simply convert the buffer to UTF-8 string, if the buffer encoding is not utf8, this would make the final request body incorrect

what's the best way of doing it?

I found that VSCode interally use the library jschardet to guess textual file content encoding (https://github.com/microsoft/vscode/blob/ce1462776f7ada735ba82ffec28ea774e617a927/src/vs/workbench/services/textfile/common/encoding.ts#L234). Together with the following advice, if we add the request level setting for this feature, maybe we can also support providing an optional argument encoding, if omit, we will guess the encoding for him/her. By default the setting is turned off.

think we can default to utf8 and provide a way to override the default to keep it simple

added those changes, can you check?

Huachao · 2020-07-06T01:57:15Z

src/utils/httpRequestParser.ts

+                            const resolvedContent = await VariableProcessor.processRawRequest(fileContent);
+                            combinedStream.append(resolvedContent);


If we really want to support this feature, I think we should add a switch(setting) to turn on/off this behavior that parsing the variables in the included body file.

overall setting? that seems a bit excessive, why not a request level setting?

like instead of < filename we could do <@ filename to imply it contains variables needing resolving ? What do you think @Huachao

added this with an optional encoding, seems like a nice option, brings in the default with an option to override and only processes variables in files if we signal it that way with the @

nice suggestion

Huachao · 2020-07-06T08:23:10Z

src/utils/httpRequestParser.ts

@@ -179,7 +180,10 @@ export class HttpRequestParser implements RequestParser {
                        const inputFilePath = groups[1];
                        const fileAbsolutePath = await resolveRequestBodyPath(inputFilePath);
                        if (fileAbsolutePath) {
-                            combinedStream.append(fs.createReadStream(fileAbsolutePath));
+                            const buffer = await fs.readFile(fileAbsolutePath);
+                            const fileContent = buffer.toString('utf8');


I found that VSCode interally use the library jschardet to guess textual file content encoding (https://github.com/microsoft/vscode/blob/ce1462776f7ada735ba82ffec28ea774e617a927/src/vs/workbench/services/textfile/common/encoding.ts#L234). Together with the following advice, if we add the request level setting for this feature, maybe we can also support providing an optional argument encoding, if omit, we will guess the encoding for him/her. By default the setting is turned off.

@Latin1

…flagged when referencing the file via an @ sign '<@' and optionally the encoding name (utf8 by default) '<@Latin1'

Huachao · 2020-07-06T23:31:46Z

src/utils/httpRequestParser.ts

+                                const resolvedContent = await VariableProcessor.processRawRequest(fileContent);
+                                combinedStream.append(resolvedContent);
+                            }
+                            else {


Please fix the tslint error

Huachao · 2020-07-08T01:20:01Z

src/utils/httpRequestParser.ts

@@ -21,7 +22,8 @@ enum ParseState {
 export class HttpRequestParser implements RequestParser {
    private readonly defaultMethod = 'GET';
    private readonly queryStringLinePrefix = /^\s*[&\?]/;
-    private readonly inputFileSyntax = /^<\s+(.+?)\s*$/;


The documentLInkProvider.ts also uses the same regex to identify the file link, you also need to update the regex there since the Click link feature is broken when appendig @ after <

very good point, noticed the underscore stopped working after my change!

Huachao · 2020-07-08T08:32:26Z

src/providers/documentLinkProvider.ts

@@ -7,7 +7,7 @@ import { getWorkspaceRootPath } from '../utils/workspaceUtility';

 export class RequestBodyDocumentLinkProvider implements DocumentLinkProvider {

-    private _linkPattern = /^(\<\s+)(.+)(\s*)$/g;
+    private _linkPattern = /^(\<(?:@?(?:\w+)?)?\s+)(.+)(\s*)$/g;


It seems in your regex, the encoding is supported with the < syntax, in my option, it's only supported with the <@ syntax

Huachao · 2020-07-08T09:47:00Z

@ricmrodrigues Merged, thanks

jackbravo · 2020-07-10T03:38:43Z

What was missing was documenting how this feature works in the README file.

ricmrodrigues · 2020-07-10T11:48:46Z

What was missing was documenting how this feature works in the README file.

#637

Huachao · 2020-08-30T03:21:32Z

@ricmrodrigues you can try the latest version 0.24.2

processing variables from external file

d0a0fa7

Huachao requested changes Jul 6, 2020

View reviewed changes

adding syntax so that external file variable parsing is optional and …

594b492

…flagged when referencing the file via an @ sign '<@' and optionally the encoding name (utf8 by default) '<@Latin1'

Huachao reviewed Jul 6, 2020

View reviewed changes

fixed tslint issue

391bcc5

Huachao requested changes Jul 8, 2020

View reviewed changes

updated link for file clicking

5cbc8f9

Huachao requested changes Jul 8, 2020

View reviewed changes

fixed regex to not support encoding without the @

7d4cbc3

Huachao approved these changes Jul 8, 2020

View reviewed changes

Huachao merged commit a204832 into Huachao:master Jul 8, 2020

ricmrodrigues deleted the adding-external-file-body-variables branch July 8, 2020 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

processing variables from external file #632

processing variables from external file #632

ricmrodrigues commented Jul 3, 2020

Huachao Jul 6, 2020

ricmrodrigues Jul 6, 2020

Huachao Jul 6, 2020

ricmrodrigues Jul 6, 2020

ricmrodrigues Jul 6, 2020

Huachao Jul 6, 2020

ricmrodrigues Jul 6, 2020

ricmrodrigues Jul 6, 2020 •

edited

Loading

ricmrodrigues Jul 6, 2020

Huachao Jul 7, 2020

Huachao Jul 6, 2020

Huachao Jul 6, 2020

ricmrodrigues Jul 7, 2020

Huachao Jul 8, 2020

ricmrodrigues Jul 8, 2020

Huachao Jul 8, 2020

ricmrodrigues Jul 8, 2020

Huachao commented Jul 8, 2020

jackbravo commented Jul 10, 2020

ricmrodrigues commented Jul 10, 2020

Huachao commented Aug 30, 2020

		const resolvedContent = await VariableProcessor.processRawRequest(fileContent);
		combinedStream.append(resolvedContent);

processing variables from external file #632

processing variables from external file #632

Conversation

ricmrodrigues commented Jul 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricmrodrigues Jul 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Huachao commented Jul 8, 2020

jackbravo commented Jul 10, 2020

ricmrodrigues commented Jul 10, 2020

Huachao commented Aug 30, 2020

ricmrodrigues Jul 6, 2020 •

edited

Loading