Skip to content

Commit

Permalink
[8.x] [Security Solution][Detection Engine] Avoid creating list items…
Browse files Browse the repository at this point in the history 
… for empty lines in import list API (elastic#192681) (elastic#194470)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Detection Engine] Avoid creating list items for
empty lines in import list API
(elastic#192681)](elastic#192681)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"55718608+marshallmain@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-30T18:07:39Z","message":"[Security
Solution][Detection Engine] Avoid creating list items for empty lines in
import list API (elastic#192681)\n\n## Summary\r\n\r\nThe quickstart tooling
introduced in\r\nhttps://github.com/elastic/pull/190634 uses
axios under the hood\r\nto make requests to Kibana. When attaching file
data to the axios\r\nrequest with `FormData`, axios adds an extra empty
line after the end\r\ncontent boundary. The logic in `buffer_lines.ts`
assumes that there are\r\nno more lines after the end content boundary
line, so importing a list\r\nwith the quickstart tooling would create a
list with an extra empty\r\nitem. This empty item fails validation when
retrieved through other\r\nAPIs.\r\n\r\nThis PR prevents lines after the
end content boundary from being turned\r\ninto list items in the import
list
API.","sha":"5f83ac05991cd980ef5b205acd19c997b60045a3","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Team:Detection
Engine","v8.16.0"],"title":"[Security Solution][Detection Engine] Avoid
creating list items for empty lines in import list
API","number":192681,"url":"https://github.com/elastic/kibana/pull/192681","mergeCommit":{"message":"[Security
Solution][Detection Engine] Avoid creating list items for empty lines in
import list API (elastic#192681)\n\n## Summary\r\n\r\nThe quickstart tooling
introduced in\r\nhttps://github.com/elastic/pull/190634 uses
axios under the hood\r\nto make requests to Kibana. When attaching file
data to the axios\r\nrequest with `FormData`, axios adds an extra empty
line after the end\r\ncontent boundary. The logic in `buffer_lines.ts`
assumes that there are\r\nno more lines after the end content boundary
line, so importing a list\r\nwith the quickstart tooling would create a
list with an extra empty\r\nitem. This empty item fails validation when
retrieved through other\r\nAPIs.\r\n\r\nThis PR prevents lines after the
end content boundary from being turned\r\ninto list items in the import
list
API.","sha":"5f83ac05991cd980ef5b205acd19c997b60045a3"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192681","number":192681,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Avoid creating list items for empty lines in
import list API (elastic#192681)\n\n## Summary\r\n\r\nThe quickstart tooling
introduced in\r\nhttps://github.com/elastic/pull/190634 uses
axios under the hood\r\nto make requests to Kibana. When attaching file
data to the axios\r\nrequest with `FormData`, axios adds an extra empty
line after the end\r\ncontent boundary. The logic in `buffer_lines.ts`
assumes that there are\r\nno more lines after the end content boundary
line, so importing a list\r\nwith the quickstart tooling would create a
list with an extra empty\r\nitem. This empty item fails validation when
retrieved through other\r\nAPIs.\r\n\r\nThis PR prevents lines after the
end content boundary from being turned\r\ninto list items in the import
list
API.","sha":"5f83ac05991cd980ef5b205acd19c997b60045a3"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <55718608+marshallmain@users.noreply.github.com>
  • Loading branch information
@kibanamachine @marshallmain
kibanamachine and marshallmain authored Sep 30, 2024
1 parent 08d5c08 commit 4c6f2e9
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 61 deletions.
92 changes: 32 additions & 60 deletions x-pack/plugins/lists/server/services/items/buffer_lines.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@ describe('buffer_lines', () => {
}).toThrow('bufferSize must be greater than zero');
});

test('it can read a single line', (done) => {
test('two identical lines are collapsed into just one line without duplicates', (done) => {
const input = new TestReadable();
input.push('--boundary\n');
input.push('Content-type: text/plain\n');
input.push('Content-Disposition: form-data; name="fieldName"; filename="filename.text"\n');
input.push('\n');
input.push('line one\n');
input.push('line one\n');
input.push('--boundary--\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
let linesToTest: string[] = [];
Expand All @@ -38,100 +44,65 @@ describe('buffer_lines', () => {
});
});

test('it can read a single line using a buffer size of 1', (done) => {
test('it can close out without writing any lines', (done) => {
const input = new TestReadable();
input.push('line one\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: 1, input });
let linesToTest: string[] = [];
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest).toEqual(['line one']);
done();
});
});

test('it can read two lines', (done) => {
const input = new TestReadable();
input.push('line one\n');
input.push('line two\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
let linesToTest: string[] = [];
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest).toEqual(['line one', 'line two']);
done();
});
});

test('it can read two lines using a buffer size of 1', (done) => {
const input = new TestReadable();
input.push('line one\n');
input.push('line two\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: 1, input });
let linesToTest: string[] = [];
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest).toEqual(['line one', 'line two']);
expect(linesToTest).toEqual([]);
done();
});
});

test('two identical lines are collapsed into just one line without duplicates', (done) => {
test('it can read 200 lines', (done) => {
const input = new TestReadable();
input.push('line one\n');
input.push('line one\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
input.push('--boundary\n');
input.push('Content-type: text/plain\n');
input.push('Content-Disposition: form-data; name="fieldName"; filename="filename.text"\n');
input.push('\n');
let linesToTest: string[] = [];
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest).toEqual(['line one']);
done();
});
});

test('it can close out without writing any lines', (done) => {
const input = new TestReadable();
const size200: string[] = new Array(200).fill(null).map((_, index) => `${index}\n`);
size200.forEach((element) => input.push(element));
input.push('--boundary--\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
let linesToTest: string[] = [];
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest).toEqual([]);
expect(linesToTest.length).toEqual(200);
done();
});
});

test('it can read 200 lines', (done) => {
test('it can read an example multi-part message', (done) => {
const input = new TestReadable();
input.push('--boundary\n');
input.push('Content-type: text/plain\n');
input.push('Content-Disposition: form-data; name="fieldName"; filename="filename.text"\n');
input.push('\n');
input.push('127.0.0.1\n');
input.push('127.0.0.2\n');
input.push('127.0.0.3\n');
input.push('\n');
input.push('--boundary--\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
let linesToTest: string[] = [];
const size200: string[] = new Array(200).fill(null).map((_, index) => `${index}\n`);
size200.forEach((element) => input.push(element));
input.push(null);
bufferedLine.on('lines', (lines: string[]) => {
linesToTest = [...linesToTest, ...lines];
});
bufferedLine.on('close', () => {
expect(linesToTest.length).toEqual(200);
expect(linesToTest).toEqual(['127.0.0.1', '127.0.0.2', '127.0.0.3']);
done();
});
});

test('it can read an example multi-part message', (done) => {
test('it does not create empty values for lines after the end boundary', (done) => {
const input = new TestReadable();
input.push('--boundary\n');
input.push('Content-type: text/plain\n');
Expand All @@ -142,6 +113,7 @@ describe('buffer_lines', () => {
input.push('127.0.0.3\n');
input.push('\n');
input.push('--boundary--\n');
input.push('\n');
input.push(null);
const bufferedLine = new BufferLines({ bufferSize: IMPORT_BUFFER_SIZE, input });
let linesToTest: string[] = [];
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lists/server/services/items/buffer_lines.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class BufferLines extends Readable {
// we are at the end of the stream
this.boundary = null;
this.readableText = false;
} else {
} else if (this.readableText) {
// we have actual content to push
this.push(line);
}
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/lists/server/services/items/write_lines_to_bulk_list_items.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ describe('write_lines_to_bulk_list_items', () => {
test('It imports a set of items to a write buffer by calling "getListItemByValues" with a single value given', async () => {
const options = getImportListItemsToStreamOptionsMock();
const promise = importListItemsToStream(options);
options.stream.push('--boundary\n');
options.stream.push('Content-type: text/plain\n');
options.stream.push(
'Content-Disposition: form-data; name="fieldName"; filename="filename.text"\n'
);
options.stream.push('\n');
options.stream.push('127.0.0.1\n');
options.stream.push(null);
await promise;
Expand All @@ -58,6 +64,12 @@ describe('write_lines_to_bulk_list_items', () => {
test('It imports a set of items to a write buffer by calling "getListItemByValues" with two values given', async () => {
const options = getImportListItemsToStreamOptionsMock();
const promise = importListItemsToStream(options);
options.stream.push('--boundary\n');
options.stream.push('Content-type: text/plain\n');
options.stream.push(
'Content-Disposition: form-data; name="fieldName"; filename="filename.text"\n'
);
options.stream.push('\n');
options.stream.push('127.0.0.1\n');
options.stream.push('127.0.0.2\n');
options.stream.push(null);
Expand Down

0 comments on commit 4c6f2e9

Please sign in to comment.