suggestions for "Improve /etc/hosts generation"#1
Closed
thaJeztah wants to merge 7 commits intorobmry:simplify_etchostsfrom
Closed
suggestions for "Improve /etc/hosts generation"#1thaJeztah wants to merge 7 commits intorobmry:simplify_etchostsfrom
thaJeztah wants to merge 7 commits intorobmry:simplify_etchostsfrom
Conversation
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
This regex is constructed using user-input, which could technically produce an invalid regex. Given that we have an error-return to our availability, let's return any error we get, instead of panicking. Signed-off-by: Sebastiaan van Stijn <[email protected]>
The `mergeRecords` function wasn't actually _merging_ anything, but only appended records to the existing `/etc/hosts` content. However, doing so was split across two functions; `Add` and `mergeRecords()`; - `Add()` obtains a lock for the given path - then calls `mergeRecords` which reads the file-content and appends the new records to the content. - Closes the file and returns the new content - Then `Add` does a `os.WriteFile` to ... the same file Given that we're appending, we won't have to read the file's content, and we can append to the file itself. Signed-off-by: Sebastiaan van Stijn <[email protected]>
We already have the filehandle open, so we could just truncate, and overwrite the content. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Call the respective (`etchosts.BuildNoIPv6` or `etchosts.Build`) functions directly instead of using the intermediate `buildf` variable. Signed-off-by: Sebastiaan van Stijn <[email protected]>
robmry
force-pushed
the
simplify_etchosts
branch
2 times, most recently
from
5e5cd8c to
7c1e41a
Compare
Owner
|
Merged to master in moby#48823 |
robmry
pushed a commit
that referenced
this pull request
Jan 2, 2025
contains a fix for CVE-2024-45338 / https://go.dev/issue/70906, but it doesn't affect our codebase: govulncheck -show=verbose ./... Scanning your code and 1260 packages across 211 dependent modules for known vulnerabilities... ... Vulnerability #1: GO-2024-3333 Non-linear parsing of case-insensitive content in golang.org/x/net/html More info: https://pkg.go.dev/vuln/GO-2024-3333 Module: golang.org/x/net Found in: golang.org/x/[email protected] Fixed in: golang.org/x/[email protected] Your code is affected by 0 vulnerabilities. This scan also found 0 vulnerabilities in packages you import and 1 vulnerability in modules you require, but your code doesn't appear to call these vulnerabilities. full diff: golang/net@v0.32.0...v0.33.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
libnetwork/etchosts: Add: combine with "mergeRecords()"
The
mergeRecordsfunction wasn't actually merging anything, but onlyappended records to the existing
/etc/hostscontent. However, doing sowas split across two functions;
AddandmergeRecords();Add()obtains a lock for the given pathmergeRecordswhich reads the file-content and appends thenew records to the content.
Adddoes aos.WriteFileto ... the same fileGiven that we're appending, we won't have to read the file's content, and
we can append to the file itself.
libnetwork/etchosts: Delete: truncate file instead of close and write
We already have the filehandle open, so we could just truncate, and
overwrite the content.
libnetwork: Sandbox.buildHostsFile: rename var that shadowed type
libnetwork: Sandbox.buildHostsFile: remove intermediate var
Call the respective (
etchosts.BuildNoIPv6oretchosts.Build) functionsdirectly instead of using the intermediate
buildfvariable.