Skip to content
This repository was archived by the owner on Mar 12, 2020. It is now read-only.

ParseLicense: fix incorrectly stripping BOM#29

Open
thaJeztah wants to merge 1 commit into
docker-archive:masterfrom
thaJeztah:fix_bom_stripping
Open

ParseLicense: fix incorrectly stripping BOM#29
thaJeztah wants to merge 1 commit into
docker-archive:masterfrom
thaJeztah:fix_bom_stripping

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

bytes.Trim() takes a cutset as argument, which means that any character occurring in the cutset will be removed from the start (and end) of the input.

For example (replacing the BOM characters with readable characters):

license := []byte("BOMMOBhelloBOMMOB")
parsed := bytes.Trim(license, "BOM")
fmt.Println(string(parsed))

Will produce:

hello

This patch updates the function to use bytes.TrimPrefix() which takes a prefix as argument, and only removes exact occurences of the given prefix:

license := []byte("BOMMOBhelloBOMMOB")
parsed = bytes.TrimPrefix(license, []byte("BOM"))
fmt.Println(string(parsed))

Will produce:

MOBhelloBOMMOB

Also see https://play.golang.org/p/H_cFZ80miNV for an example

`bytes.Trim()` takes a cutset as argument, which means that any
character occurring in the cutset will be removed from the start
(and end) of the input.

For example (replacing the BOM characters with readable characters):

```go
license := []byte("BOMMOBhelloBOMMOB")
parsed := bytes.Trim(license, "BOM")
fmt.Println(string(parsed))
```

Will produce:

```
hello
```

This patch updates the function to use `bytes.TrimPrefix()` which
takes a `prefix` as argument, and only removes exact occurrences of
the given prefix:

```go
license := []byte("BOMMOBhelloBOMMOB")
parsed = bytes.TrimPrefix(license, []byte("BOM"))
fmt.Println(string(parsed))
```

Will produce:

```
MOBhelloBOMMOB
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Copy Markdown
Member Author

ping @mason-fish @dhiltgen PTAL

@andrewhsu andrewhsu requested review from dhiltgen and mason-fish July 31, 2019 01:21
Copy link
Copy Markdown
Contributor

@dhiltgen dhiltgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding more test coverage on this part.

The new code is better/more-correct, but given the license server will never stuff a BOM on the end, it's somewhat academic. The change seems fine to me.

@thaJeztah
Copy link
Copy Markdown
Member Author

@dhiltgen @mason-fish good to go?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants