Skip to content

Commit c8b3683

Browse files
committed
Check that the directory exists before applying operations.
This yields some nicer error messages rather than letting the failure manifest somewhere down the line, e.g., from lock timeouts.
1 parent e273e5c commit c8b3683

9 files changed

+151
-11
lines changed

create.go

+3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func createProject(project string, inperms *unsafePermissionsMetadata, req_user
5757

5858
// No need to lock before MkdirAll, it just no-ops if the directory already exists.
5959
err = os.MkdirAll(project_dir, 0755)
60+
if err != nil {
61+
return fmt.Errorf("failed to create a new project directory for %s; %w", project, err)
62+
}
6063

6164
globals.Locks.LockDirectory(project_dir, 10 * time.Second)
6265
if err != nil {

latest.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,26 @@ func refreshLatestHandler(reqpath string, globals *globalConfiguration) (*latest
120120
}
121121
}
122122

123+
project := *(incoming.Project)
124+
project_dir := filepath.Join(globals.Registry, project)
125+
if err := checkProjectExists(project_dir, project); err != nil {
126+
return nil, err
127+
}
128+
123129
// Technically we only need a lock on the asset directory, but all
124130
// mutating operations will lock the project directory, so we respect that.
125-
project_dir := filepath.Join(globals.Registry, *(incoming.Project))
126131
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
127132
if err != nil {
128133
return nil, fmt.Errorf("failed to acquire the lock on the project directory %q; %w", project_dir, err)
129134
}
130135
defer globals.Locks.Unlock(project_dir)
131136

132-
asset_dir := filepath.Join(project_dir, *(incoming.Asset))
137+
asset := *(incoming.Asset)
138+
asset_dir := filepath.Join(project_dir, asset)
139+
if err := checkAssetExists(asset_dir, asset, project); err != nil {
140+
return nil, err
141+
}
142+
133143
output, err := refreshLatest(asset_dir)
134144
return output, err
135145
}

permissions.go

+4
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,10 @@ func setPermissionsHandler(reqpath string, globals *globalConfiguration) error {
221221

222222
project := *(incoming.Project)
223223
project_dir := filepath.Join(globals.Registry, project)
224+
if err := checkProjectExists(project_dir, project); err != nil {
225+
return err
226+
}
227+
224228
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
225229
if err != nil {
226230
return fmt.Errorf("failed to lock project directory %q; %w", project_dir, err)

probation.go

+18-4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ func baseProbationHandler(reqpath string, globals *globalConfiguration, approve
5050

5151
project := *(incoming.Project)
5252
project_dir := filepath.Join(globals.Registry, project)
53+
if err := checkProjectExists(project_dir, project); err != nil {
54+
return err
55+
}
56+
5357
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
5458
if err != nil {
5559
return fmt.Errorf("failed to lock project directory %q; %w", project_dir, err)
@@ -64,8 +68,18 @@ func baseProbationHandler(reqpath string, globals *globalConfiguration, approve
6468
return newHttpError(http.StatusForbidden, fmt.Errorf("user %q is not authorized to modify probation status in %q", username, project_dir))
6569
}
6670

67-
asset_dir := filepath.Join(project_dir, *(incoming.Asset))
68-
version_dir := filepath.Join(asset_dir, *(incoming.Version))
71+
asset := *(incoming.Asset)
72+
asset_dir := filepath.Join(project_dir, asset)
73+
if err := checkAssetExists(asset_dir, asset, project); err != nil {
74+
return err
75+
}
76+
77+
version := *(incoming.Version)
78+
version_dir := filepath.Join(asset_dir, version)
79+
if err := checkVersionExists(version_dir, version, asset, project); err != nil {
80+
return err
81+
}
82+
6983
summ, err := readSummary(version_dir)
7084
if err != nil {
7185
return fmt.Errorf("failed to read the version summary at %q; %w", version_dir, err)
@@ -120,8 +134,8 @@ func baseProbationHandler(reqpath string, globals *globalConfiguration, approve
120134
log_info := map[string]interface{} {
121135
"type": "add-version",
122136
"project": project,
123-
"asset": *(incoming.Asset),
124-
"version": *(incoming.Version),
137+
"asset": asset,
138+
"version": version,
125139
"latest": overwrite_latest,
126140
}
127141
err = dumpLog(globals.Registry, &log_info)

reindex.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ func reindexHandler(reqpath string, globals *globalConfiguration) error {
7474
// Configuring the project; we apply a lock to the project to avoid concurrent changes.
7575
project := *(request.Project)
7676
project_dir := filepath.Join(globals.Registry, project)
77+
if err := checkProjectExists(project_dir, project); err != nil {
78+
return err
79+
}
80+
7781
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
7882
if err != nil {
7983
return fmt.Errorf("failed to acquire the lock on %q; %w", project_dir, err)
@@ -94,14 +98,14 @@ func reindexHandler(reqpath string, globals *globalConfiguration) error {
9498
// Configuring the asset and version.
9599
asset := *(request.Asset)
96100
asset_dir := filepath.Join(project_dir, asset)
97-
if _, err := os.Stat(asset_dir); err != nil {
98-
return newHttpError(http.StatusNotFound, fmt.Errorf("cannot access asset directory at %q; %w", asset_dir, err))
101+
if err := checkAssetExists(asset_dir, asset, project); err != nil {
102+
return err
99103
}
100104

101105
version := *(request.Version)
102106
version_dir := filepath.Join(asset_dir, version)
103-
if _, err := os.Stat(version_dir); err != nil {
104-
return newHttpError(http.StatusNotFound, fmt.Errorf("cannot access version directory at %q; %w", asset_dir, err))
107+
if err := checkVersionExists(version_dir, version, asset, project); err != nil {
108+
return err
105109
}
106110

107111
err = reindexDirectory(globals.Registry, project, asset, version)

upload.go

+4
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ func uploadHandler(reqpath string, globals *globalConfiguration) error {
9898
// Configuring the project; we apply a lock to the project to avoid concurrent changes.
9999
project := *(request.Project)
100100
project_dir := filepath.Join(globals.Registry, project)
101+
if err := checkProjectExists(project_dir, project); err != nil {
102+
return err
103+
}
104+
101105
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
102106
if err != nil {
103107
return fmt.Errorf("failed to acquire the lock on %q; %w", project_dir, err)

usage.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ func refreshUsageHandler(reqpath string, globals *globalConfiguration) (*usageMe
108108
}
109109
}
110110

111-
project_dir := filepath.Join(globals.Registry, *(incoming.Project))
111+
project := *(incoming.Project)
112+
project_dir := filepath.Join(globals.Registry, project)
113+
if err := checkProjectExists(project_dir, project); err != nil {
114+
return nil, err
115+
}
116+
112117
err = globals.Locks.LockDirectory(project_dir, 10 * time.Second)
113118
if err != nil {
114119
return nil, fmt.Errorf("failed to lock the project directory %q; %w", project_dir, err)

utils.go

+34
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"math/rand"
1111
"strconv"
1212
"path/filepath"
13+
"net/http"
1314
)
1415

1516
type globalConfiguration struct {
@@ -114,3 +115,36 @@ func dumpLog(registry string, content interface{}) error {
114115
path := time.Now().Format(time.RFC3339) + "_" + strconv.Itoa(100000 + rand.Intn(900000))
115116
return dumpJson(filepath.Join(registry, logDirName, path), content)
116117
}
118+
119+
func checkProjectExists(project_dir, project string) error {
120+
_, err := os.Stat(project_dir)
121+
if errors.Is(err, os.ErrNotExist) {
122+
return newHttpError(http.StatusNotFound, fmt.Errorf("project %s does not exist", project))
123+
} else if err != nil {
124+
return fmt.Errorf("failed to stat %q; %w", project_dir, err)
125+
} else {
126+
return nil
127+
}
128+
}
129+
130+
func checkAssetExists(asset_dir, asset, project string) error {
131+
_, err := os.Stat(asset_dir)
132+
if errors.Is(err, os.ErrNotExist) {
133+
return newHttpError(http.StatusNotFound, fmt.Errorf("asset %s does not exist in project %s", asset, project))
134+
} else if err != nil {
135+
return fmt.Errorf("failed to stat %q; %w", asset_dir, err)
136+
} else {
137+
return nil
138+
}
139+
}
140+
141+
func checkVersionExists(version_dir, version, asset, project string) error {
142+
_, err := os.Stat(version_dir)
143+
if errors.Is(err, os.ErrNotExist) {
144+
return newHttpError(http.StatusNotFound, fmt.Errorf("version %s does not exist for asset %s of project %s", version, asset, project))
145+
} else if err != nil {
146+
return fmt.Errorf("failed to stat %q; %w", version_dir, err)
147+
} else {
148+
return nil
149+
}
150+
}

utils_test.go

+62
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"fmt"
88
"path/filepath"
99
"encoding/json"
10+
"errors"
11+
"net/http"
1012
)
1113

1214
func dumpRequest(request_type, request_string string) (string, error) {
@@ -109,3 +111,63 @@ func TestIsBadName(t *testing.T) {
109111
t.Fatal("failed to stop in the presence of a backslash")
110112
}
111113
}
114+
115+
func TestCheckProjectExists(t *testing.T) {
116+
reg, err := os.MkdirTemp("", "")
117+
if err != nil {
118+
t.Fatal(err)
119+
}
120+
121+
err = checkProjectExists(filepath.Join(reg, "doesnt_exist"), "foo")
122+
var http_err *httpError
123+
if !errors.As(err, &http_err) {
124+
t.Error("expected a HTTP error")
125+
} else if http_err.Status != http.StatusNotFound {
126+
t.Error("expected a HTTP error with a 404")
127+
}
128+
129+
err = checkProjectExists(reg, "foo")
130+
if err != nil {
131+
t.Error("no error expected here")
132+
}
133+
}
134+
135+
func TestCheckAssetExists(t *testing.T) {
136+
reg, err := os.MkdirTemp("", "")
137+
if err != nil {
138+
t.Fatal(err)
139+
}
140+
141+
err = checkAssetExists(filepath.Join(reg, "doesnt_exist"), "bar", "foo")
142+
var http_err *httpError
143+
if !errors.As(err, &http_err) {
144+
t.Error("expected a HTTP error")
145+
} else if http_err.Status != http.StatusNotFound {
146+
t.Error("expected a HTTP error with a 404")
147+
}
148+
149+
err = checkAssetExists(reg, "bar", "foo")
150+
if err != nil {
151+
t.Error("no error expected here")
152+
}
153+
}
154+
155+
func TestCheckVersionExists(t *testing.T) {
156+
reg, err := os.MkdirTemp("", "")
157+
if err != nil {
158+
t.Fatal(err)
159+
}
160+
161+
err = checkVersionExists(filepath.Join(reg, "doesnt_exist"), "whee", "bar", "foo")
162+
var http_err *httpError
163+
if !errors.As(err, &http_err) {
164+
t.Error("expected a HTTP error")
165+
} else if http_err.Status != http.StatusNotFound {
166+
t.Error("expected a HTTP error with a 404")
167+
}
168+
169+
err = checkVersionExists(reg, "whee", "bar", "foo")
170+
if err != nil {
171+
t.Error("no error expected here")
172+
}
173+
}

0 commit comments

Comments
 (0)