Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tmc): panic due to index out of range in logsyncer #1189

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

i4ki
Copy link
Contributor

@i4ki i4ki commented Oct 20, 2023

Reasons For This Change

When providing an stdout/stderr containing an empty line, the log syncer crashes:

panic: runtime error: index out of range [-1]

goroutine 213 [running]:
github.com/terramate-io/terramate/cloud.dropCRLN(...)
        /vagrant/terramate/cloud/log_syncer.go:192
github.com/terramate-io/terramate/cloud.(*LogSyncer).NewBuffer.func1()
        /vagrant/terramate/cloud/log_syncer.go:103 +0x6fd
created by github.com/terramate-io/terramate/cloud.(*LogSyncer).NewBuffer
        /vagrant/terramate/cloud/log_syncer.go:80 +0x2a5
FAIL    github.com/terramate-io/terramate/cloud 0.055s
FAIL

Description of Changes

A bounds check is added before removing the CRLN from the line.

## Reasons For This Change

When providing an stdout/stderr containing an empty line,
the log syncer crashes:

```
panic: runtime error: index out of range [-1]

goroutine 213 [running]:
github.com/terramate-io/terramate/cloud.dropCRLN(...)
        /vagrant/terramate/cloud/log_syncer.go:192
github.com/terramate-io/terramate/cloud.(*LogSyncer).NewBuffer.func1()
        /vagrant/terramate/cloud/log_syncer.go:103 +0x6fd
created by github.com/terramate-io/terramate/cloud.(*LogSyncer).NewBuffer
        /vagrant/terramate/cloud/log_syncer.go:80 +0x2a5
FAIL    github.com/terramate-io/terramate/cloud 0.055s
FAIL
```

### Description of Changes

A bounds check is added before removing the CRLN from the line.

Signed-off-by: Tiago Natel <t.nateldemoura@gmail.com>
@i4ki i4ki requested a review from a team as a code owner October 20, 2023 00:13
@netlify
Copy link

netlify bot commented Oct 20, 2023

Deploy Preview for terramate-io-docs canceled.

Name Link
🔨 Latest commit 5853658
🔍 Latest deploy log https://app.netlify.com/sites/terramate-io-docs/deploys/6531c62861a5470008ecebc4

@i4ki i4ki changed the title fix(tmc): panic due to index out of bounds in logsync. fix(tmc): panic due to index out of range in log syncer Oct 20, 2023
@i4ki i4ki changed the title fix(tmc): panic due to index out of range in log syncer fix(tmc): panic due to index out of range in logsyncer Oct 20, 2023
@github-actions
Copy link

metric: time/op
ListFiles-2: old 177µs ± 5%: new 174µs ± 4%: delta: -1.56%
Generate-2: old 4.34s ± 2%: new 4.47s ± 2%: delta: 3.09%
GenerateRegex-2: old 2.77s ± 3%: new 2.81s ± 3%: delta: 1.43%
TokensForExpressionComplex-2: old 2.19ms ± 3%: new 2.20ms ± 1%: delta: 0.69%
TokensForExpressionPlainStringNoNewline-2: old 1.63µs ± 1%: new 1.64µs ± 2%: delta: 0.91%
TokensForExpressionStringWith100Newlines-2: old 53.1µs ± 3%: new 53.1µs ± 3%: delta: 0.00%
TokensForExpressionObjectWith100KeysWithNumberValues-2: old 2.53ms ± 2%: new 2.54ms ± 2%: delta: 0.00%
TokensForExpression-2: old 2.19ms ± 2%: new 2.19ms ± 1%: delta: 0.00%
PartialEvalComplex-2: old 707µs ± 2%: new 712µs ± 2%: delta: 0.00%
PartialEvalSmallString-2: old 6.29µs ± 3%: new 6.21µs ± 2%: delta: -1.29%
PartialEvalHugeString-2: old 3.06ms ± 2%: new 3.05ms ± 2%: delta: 0.00%
PartialEvalHugeInterpolatedString-2: old 8.24ms ± 4%: new 8.35ms ± 4%: delta: 1.34%
PartialEvalObject-2: old 35.3µs ± 3%: new 36.2µs ± 4%: delta: 2.52%
metric: alloc/op
ListFiles-2: old 99.5kB ± 0%: new 99.5kB ± 0%: delta: 0.00%
Generate-2: old 2.34GB ± 0%: new 2.34GB ± 0%: delta: 0.00%
GenerateRegex-2: old 953MB ± 0%: new 953MB ± 0%: delta: 0.00%
TokensForExpressionComplex-2: old 412kB ± 0%: new 412kB ± 0%: delta: 0.00%
TokensForExpressionPlainStringNoNewline-2: old 592B ± 0%: new 592B ± 0%: delta: 0.00%
TokensForExpressionStringWith100Newlines-2: old 14.0kB ± 0%: new 14.0kB ± 0%: delta: 0.00%
TokensForExpressionObjectWith100KeysWithNumberValues-2: old 402kB ± 0%: new 402kB ± 0%: delta: 0.00%
TokensForExpression-2: old 412kB ± 0%: new 412kB ± 0%: delta: 0.00%
PartialEvalComplex-2: old 353kB ± 0%: new 353kB ± 0%: delta: 0.00%
PartialEvalSmallString-2: old 1.74kB ± 0%: new 1.74kB ± 0%: delta: 0.00%
PartialEvalHugeString-2: old 166kB ± 0%: new 166kB ± 0%: delta: 0.00%
PartialEvalHugeInterpolatedString-2: old 4.38MB ± 0%: new 4.38MB ± 0%: delta: 0.00%
PartialEvalObject-2: old 20.4kB ± 0%: new 20.4kB ± 0%: delta: 0.00%
metric: allocs/op
ListFiles-2: old 474 ± 0%: new 474 ± 0%: delta: 0.00%
Generate-2: old 25.9M ± 0%: new 25.9M ± 0%: delta: 0.00%
GenerateRegex-2: old 18.3M ± 0%: new 18.3M ± 0%: delta: 0.00%
TokensForExpressionComplex-2: old 4.93k ± 0%: new 4.93k ± 0%: delta: 0.00%
TokensForExpressionPlainStringNoNewline-2: old 21.0 ± 0%: new 21.0 ± 0%: delta: 0.00%
TokensForExpressionStringWith100Newlines-2: old 328 ± 0%: new 328 ± 0%: delta: 0.00%
TokensForExpressionObjectWith100KeysWithNumberValues-2: old 3.29k ± 0%: new 3.29k ± 0%: delta: 0.00%
TokensForExpression-2: old 4.93k ± 0%: new 4.93k ± 0%: delta: 0.00%
PartialEvalComplex-2: old 2.83k ± 0%: new 2.83k ± 0%: delta: 0.00%
PartialEvalSmallString-2: old 23.0 ± 0%: new 23.0 ± 0%: delta: 0.00%
PartialEvalHugeString-2: old 35.0 ± 0%: new 35.0 ± 0%: delta: 0.00%
PartialEvalHugeInterpolatedString-2: old 23.1k ± 0%: new 23.1k ± 0%: delta: 0.00%
PartialEvalObject-2: old 125 ± 0%: new 125 ± 0%: delta: 0.00%

@i4ki i4ki added this pull request to the merge queue Oct 20, 2023
Merged via the queue into main with commit 2059930 Oct 20, 2023
13 checks passed
@i4ki i4ki deleted the i4k-fix-index-out-bounds branch October 20, 2023 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants