Skip to content

Commit e842ed2

Browse files
samueltaripinsamueltaripin
committed
address comment
1 parent b5e6c95 commit e842ed2

5 files changed

Lines changed: 133 additions & 15 deletions

File tree

‎internal/ospackage/debutils/helper.go‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,16 @@ func CreateTemporaryRepository(sourcePath, repoName, arch string) (repoPath, ser
3434
return "", "", nil, fmt.Errorf("failed to get absolute path of source directory: %w", err)
3535
}
3636

37-
if _, err := os.Stat(sourcePath); os.IsNotExist(err) {
38-
return "", "", nil, fmt.Errorf("source directory does not exist: %s", sourcePath)
37+
sourceInfo, err := os.Stat(sourcePath)
38+
if err != nil {
39+
if os.IsNotExist(err) {
40+
return "", "", nil, fmt.Errorf("source directory does not exist: %s", sourcePath)
41+
}
42+
return "", "", nil, fmt.Errorf("failed to stat source directory %s: %w", sourcePath, err)
43+
}
44+
45+
if !sourceInfo.IsDir() {
46+
return "", "", nil, fmt.Errorf("source path is not a directory: %s", sourcePath)
3947
}
4048

4149
// Check if source contains DEB files
@@ -120,18 +128,29 @@ func CreateTemporaryRepository(sourcePath, repoName, arch string) (repoPath, ser
120128
os.RemoveAll(tempRepoPath)
121129
return "", "", nil, fmt.Errorf("failed to create Packages.gz file: %w", createErr)
122130
}
123-
gzWriter := gzip.NewWriter(gzFile)
124-
if _, writeErr := gzWriter.Write(packagesData); writeErr != nil {
125-
gzFile.Close()
126-
os.RemoveAll(tempRepoPath)
127-
return "", "", nil, fmt.Errorf("failed to write Packages.gz: %w", writeErr)
128-
}
129-
if closeErr := gzWriter.Close(); closeErr != nil {
130-
gzFile.Close()
131+
if gzipErr := func() (retErr error) {
132+
defer func() {
133+
if closeErr := gzFile.Close(); closeErr != nil && retErr == nil {
134+
retErr = fmt.Errorf("failed to close Packages.gz file: %w", closeErr)
135+
}
136+
}()
137+
138+
gzWriter := gzip.NewWriter(gzFile)
139+
defer func() {
140+
if closeErr := gzWriter.Close(); closeErr != nil && retErr == nil {
141+
retErr = fmt.Errorf("failed to finalize Packages.gz: %w", closeErr)
142+
}
143+
}()
144+
145+
if _, writeErr := gzWriter.Write(packagesData); writeErr != nil {
146+
return fmt.Errorf("failed to write Packages.gz: %w", writeErr)
147+
}
148+
149+
return nil
150+
}(); gzipErr != nil {
131151
os.RemoveAll(tempRepoPath)
132-
return "", "", nil, fmt.Errorf("failed to finalize Packages.gz: %w", closeErr)
152+
return "", "", nil, gzipErr
133153
}
134-
gzFile.Close()
135154

136155
// Compute SHA256 checksums and file sizes for the Release file
137156
packagesHash, hashErr := computeFileSHA256(packagesPath)

‎internal/ospackage/debutils/helper_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,48 @@ func TestCreateTemporaryRepositoryNonExistentDirectory(t *testing.T) {
175175
}
176176
}
177177

178+
func TestCreateTemporaryRepositorySourcePathIsFile(t *testing.T) {
179+
tempDir := t.TempDir()
180+
filePath := filepath.Join(tempDir, "package1_1.0_amd64.deb")
181+
if err := os.WriteFile(filePath, []byte("fake deb content"), 0644); err != nil {
182+
t.Fatalf("failed to create source file: %v", err)
183+
}
184+
185+
_, _, _, err := CreateTemporaryRepository(filePath, "testrepo", "amd64")
186+
if err == nil {
187+
t.Fatal("expected error when source path is a file")
188+
}
189+
if !strings.Contains(err.Error(), "source path is not a directory") {
190+
t.Errorf("expected non-directory source path error, got: %v", err)
191+
}
192+
}
193+
194+
func TestCreateTemporaryRepositoryStatError(t *testing.T) {
195+
tempDir := t.TempDir()
196+
blockedParent := filepath.Join(tempDir, "blocked")
197+
if err := os.Mkdir(blockedParent, 0755); err != nil {
198+
t.Fatalf("failed to create blocked parent directory: %v", err)
199+
}
200+
201+
blockedPath := filepath.Join(blockedParent, "source")
202+
if err := os.Chmod(blockedParent, 0); err != nil {
203+
t.Fatalf("failed to restrict blocked parent permissions: %v", err)
204+
}
205+
defer os.Chmod(blockedParent, 0755)
206+
207+
if _, statErr := os.Stat(blockedPath); statErr == nil || os.IsNotExist(statErr) {
208+
t.Skip("unable to induce non-not-exist os.Stat error on this platform")
209+
}
210+
211+
_, _, _, err := CreateTemporaryRepository(blockedPath, "testrepo", "amd64")
212+
if err == nil {
213+
t.Fatal("expected stat error for inaccessible source path")
214+
}
215+
if !strings.Contains(err.Error(), "failed to stat source directory") {
216+
t.Errorf("expected stat failure error, got: %v", err)
217+
}
218+
}
219+
178220
// TestCreateTemporaryRepositoryNoDEBFiles tests error handling when source directory contains no DEB files
179221
func TestCreateTemporaryRepositoryNoDEBFiles(t *testing.T) {
180222
// Create temporary directory without DEB files

‎internal/ospackage/rpmutils/helper.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -861,8 +861,16 @@ func CreateTemporaryRepository(sourcePath, repoName string) (repoPath, serverURL
861861
return "", "", nil, fmt.Errorf("failed to get absolute path of source directory: %w", err)
862862
}
863863

864-
if _, err := os.Stat(sourcePath); os.IsNotExist(err) {
865-
return "", "", nil, fmt.Errorf("source directory does not exist: %s", sourcePath)
864+
sourceInfo, err := os.Stat(sourcePath)
865+
if err != nil {
866+
if os.IsNotExist(err) {
867+
return "", "", nil, fmt.Errorf("source directory does not exist: %s", sourcePath)
868+
}
869+
return "", "", nil, fmt.Errorf("failed to stat source directory %s: %w", sourcePath, err)
870+
}
871+
872+
if !sourceInfo.IsDir() {
873+
return "", "", nil, fmt.Errorf("source path is not a directory: %s", sourcePath)
866874
}
867875

868876
// Check if source contains RPM files

‎internal/ospackage/rpmutils/helper_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,6 +1412,48 @@ func TestRPMCreateTemporaryRepositoryNonExistentDir(t *testing.T) {
14121412
}
14131413
}
14141414

1415+
func TestRPMCreateTemporaryRepositorySourcePathIsFile(t *testing.T) {
1416+
tmpDir := t.TempDir()
1417+
filePath := filepath.Join(tmpDir, "pkg.rpm")
1418+
if err := os.WriteFile(filePath, []byte("fake rpm"), 0644); err != nil {
1419+
t.Fatalf("failed to create source file: %v", err)
1420+
}
1421+
1422+
_, _, _, err := CreateTemporaryRepository(filePath, "myrepo")
1423+
if err == nil {
1424+
t.Fatal("expected error when source path is a file")
1425+
}
1426+
if !strings.Contains(err.Error(), "source path is not a directory") {
1427+
t.Errorf("expected non-directory source path error, got: %v", err)
1428+
}
1429+
}
1430+
1431+
func TestRPMCreateTemporaryRepositoryStatError(t *testing.T) {
1432+
tmpDir := t.TempDir()
1433+
blockedParent := filepath.Join(tmpDir, "blocked")
1434+
if err := os.Mkdir(blockedParent, 0755); err != nil {
1435+
t.Fatalf("failed to create blocked parent directory: %v", err)
1436+
}
1437+
1438+
blockedPath := filepath.Join(blockedParent, "source")
1439+
if err := os.Chmod(blockedParent, 0); err != nil {
1440+
t.Fatalf("failed to restrict blocked parent permissions: %v", err)
1441+
}
1442+
defer os.Chmod(blockedParent, 0755)
1443+
1444+
if _, statErr := os.Stat(blockedPath); statErr == nil || os.IsNotExist(statErr) {
1445+
t.Skip("unable to induce non-not-exist os.Stat error on this platform")
1446+
}
1447+
1448+
_, _, _, err := CreateTemporaryRepository(blockedPath, "myrepo")
1449+
if err == nil {
1450+
t.Fatal("expected stat error for inaccessible source path")
1451+
}
1452+
if !strings.Contains(err.Error(), "failed to stat source directory") {
1453+
t.Errorf("expected stat failure error, got: %v", err)
1454+
}
1455+
}
1456+
14151457
func TestRPMCreateTemporaryRepositoryNoRPMFiles(t *testing.T) {
14161458
tmpDir := t.TempDir()
14171459
// Create non-RPM files only

‎internal/utils/network/httpserver.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ import (
1212
"github.com/open-edge-platform/os-image-composer/internal/utils/logger"
1313
)
1414

15+
const (
16+
serverReadHeaderTimeout = 10 * time.Second
17+
serverReadTimeout = 30 * time.Second
18+
)
19+
1520
// ServeRepositoryHTTP starts a temporary HTTP server to serve the repository
1621
// Returns the server URL, cleanup function, and error
1722
func ServeRepositoryHTTP(repoPath string) (serverURL string, cleanup func(), err error) {
@@ -42,7 +47,9 @@ func ServeRepositoryHTTP(repoPath string) (serverURL string, cleanup func(), err
4247
mux.Handle("/", fileHandler)
4348

4449
server := &http.Server{
45-
Handler: mux,
50+
Handler: mux,
51+
ReadHeaderTimeout: serverReadHeaderTimeout,
52+
ReadTimeout: serverReadTimeout,
4653
}
4754

4855
// Start server in goroutine

0 commit comments

Comments
 (0)