Skip to content

Commit 6d080b0

Browse files
committed
path-utils: Check directory from preference is writable.
Add tests for this and to check that excessively-long path are shortened. Update docs accordingly, including specifying what "excessively long" currently means. Note: may need to remove the permissions check from `framework/private/main` if the predicate given to `preferences:set-default` could cause the preference value in the file to be overwritten.
1 parent 8305eca commit 6d080b0

File tree

4 files changed

+142
-10
lines changed

4 files changed

+142
-10
lines changed

gui-lib/framework/main.rkt

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -546,9 +546,9 @@
546546
The value of @racket[(preferences:get 'path-utils:backup-dir)]
547547
determines the directory of the resulting path.
548548
The value of this preference must be either a path
549-
satisfying @racket[complete-path?] and @racket[directory-exists?],
549+
satisfying @racket[complete-path?],
550550
in which case it is additionally checked to ensure that the
551-
directory has not been deleted since the preference was set,
551+
directory exists and is writable,
552552
or @racket[#f] (the default).
553553
When the preference contains a valid path,
554554
the resulting path will use that directory;
@@ -557,7 +557,7 @@
557557

558558
The final element of the resulting path is derived from @racket[filename].
559559
When @racket[(preferences:get 'path-utils:backup-dir)] does not specify
560-
an extant directory, the final element of @racket[filename] is
560+
a valid directory, the final element of @racket[filename] is
561561
used directly as the base for the new element.
562562
Otherwise, the base is formed by transforming the complete path to @racket[filename]
563563
(resolved, if necessary, relative to @racket[(current-directory)])
@@ -577,6 +577,13 @@
577577
may be shortened by replacing some bytes in the middle with
578578
@racket[(bytes-append separator-byte #"..." separator-byte)]
579579
(i.e. @litchar{!...!} with @litchar{!} as the @racket[separator-byte]).}]
580+
581+
@margin-note{
582+
Currently, "excessively long" is defined as 255 bytes.
583+
This is based on @hyperlink["https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath"]{
584+
a common value} for the maximum length of an individual element of an extended-length path
585+
on Windows, but the limit is currently enforced consistently on all platforms.
586+
}
580587

581588
In either case, the final path element is formed from the base
582589
in a platform-specific manner:
@@ -598,19 +605,19 @@
598605
determines the directory of the resulting path
599606
in much the same way as @racket[path-utils:generate-backup-name]
600607
with @racket['path-utils:backup-dir].
601-
When the preference value is @racket[#f] (the default) or a
602-
directory that no longer exists, the
608+
When the preference value specifies a directory that exists and
609+
is writable, the resulting path will use that directory.
610+
Otherwise, and by default, the
603611
result will use the same directory as @racket[filename]
604612
(or, when @racket[filename] is @racket[#f], the directory determined by
605613
@racket[(find-system-path 'doc-dir)]).
606-
Otherwise, the resulting path will use the directory specified by the preference.
607614

608615
When @racket[filename] is @racket[#f], the final element of the
609616
resulting path will be an automatically-generated unique name.
610617

611618
Otherwise, the final path element will be derived from @racket[filename].
612619
When @racket[(preferences:get 'path-utils:autosave-dir)] does not return
613-
an extant directory, the last element of @racket[filename] will be
620+
a valid directory, the last element of @racket[filename] will be
614621
used directly as the base for the new element.
615622
When a valid autosave directory is specified, the base will be
616623
the complete path to @racket[filename],

gui-lib/framework/private/main.rkt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,12 @@
605605
(or (not v)
606606
(and (path? v)
607607
(complete-path? v)
608-
(directory-exists? v))))
608+
(directory-exists? v)
609+
; Q: Could this cause the user-specified value to be overwritten
610+
; if we don't currently have write permissions? We probably don't
611+
; want that to happen, so maybe we should check this only when the
612+
; value is used.
613+
(memq 'write (file-or-directory-permissions v)))))
609614

610615
(define (marshall:maybe-path->bytes v)
611616
(and (path? v) (path->bytes v)))

gui-lib/framework/private/path-utils.rkt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
(let ([maybe-dir (preferences:get pref-sym)])
1515
(and maybe-dir
1616
(directory-exists? maybe-dir)
17+
(memq 'write (file-or-directory-permissions maybe-dir))
1718
maybe-dir))))
1819

1920
(define current-backup-dir

gui-test/framework/tests/path-utils.rkt

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,84 @@
88
generate-backup-name])
99
racket/file
1010
racket/contract/base
11+
racket/sequence
12+
racket/list
1113
framework/preferences)
1214

13-
;; For uniform comparisons, all tests use simplified paths normalized
14-
;; with path->directory-path when applicable.
15+
;; For uniform comparisons with equal?, all tests use simplified paths
16+
;; normalized with path->directory-path when applicable.
1517

18+
19+
;; path-base: path? -> path?
20+
;; Returns the directory of the input path (i.e. removes the final element),
21+
;; normalized for comparison with equal?
1622
(define (path-base pth)
1723
(define-values (base name dir?)
1824
(split-path pth))
1925
(path->directory-path base))
2026

27+
28+
;; remove-all-write-permissions: path? -> any
29+
;; Modifies the permissions of the given file/directory so no one can write to it
30+
(define (remove-all-write-permissions pth)
31+
(define old-permissions-bits
32+
(file-or-directory-permissions pth 'bits))
33+
(file-or-directory-permissions
34+
pth
35+
(bitwise-and old-permissions-bits
36+
(bitwise-not user-write-bit)
37+
(bitwise-not group-write-bit)
38+
(bitwise-not other-write-bit))))
39+
40+
41+
;; call-with-preference-not-writable: procedure? (-> any) -> any
42+
;; Takes a procedure produced with preferences:get/set and a thunk.
43+
;; Calls the thunk in a context where:
44+
;; (a) it ensures the preference will have the value it did
45+
;; when call-with-preference-not-writable was called, which
46+
;; is assumed to be a path, and
47+
;; (b) it removes all write permissions from the directory
48+
;; specified by the preference before calling the thunk.
49+
;; After the thunk returns (or control escapes), call-with-preference-not-writable::
50+
;; (a) restores the directory's original permissions and
51+
;; (b) restores the preference to its initial value.
52+
(define (call-with-preference-not-writable get/set-proc thunk)
53+
(define pth
54+
(get/set-proc))
55+
(define old-permissions-bits
56+
(file-or-directory-permissions pth 'bits))
57+
(dynamic-wind
58+
(λ ()
59+
(get/set-proc pth)
60+
(remove-all-write-permissions pth))
61+
thunk
62+
(λ ()
63+
(file-or-directory-permissions pth old-permissions-bits)
64+
(get/set-proc pth))))
65+
66+
67+
;; See framework/private/path-utils for rationale
68+
(define max-path-element-length
69+
255)
70+
71+
72+
;
73+
;
74+
;
75+
;
76+
; ;; ;;
77+
; ;; ;;
78+
; ;;;;;;; ;;; ;; ;;;;;;; ;;
79+
; ;; ;; ; ;; ; ;; ;; ;
80+
; ;; ; ; ; ;; ;
81+
; ;; ;;;;;;;; ;; ;; ;;
82+
; ;; ; ;; ;; ;;
83+
; ; ;; ; ; ; ; ; ;
84+
; ;;; ;;; ;;; ;;; ;;;
85+
;
86+
;
87+
;
88+
;
2189
(let ([the-prefs-table (make-hash)])
2290
(parameterize ([preferences:low-level-put-preferences
2391
(λ (syms vals)
@@ -42,6 +110,21 @@
42110
(build-path current-dir "somewhere" elem))
43111
(define dir-of-complete
44112
(path-base complete))
113+
(define too-long-pth
114+
(build-path
115+
current-dir
116+
(bytes->path-element
117+
(bytes-append
118+
(list->bytes
119+
(for/list ([i (in-range (+ 10 max-path-element-length))]
120+
[b (in-cycle
121+
(sequence-filter (compose1 (λ (c)
122+
(or (char-alphabetic? c)
123+
(char-numeric? c)))
124+
integer->char)
125+
(in-range 48 123)))])
126+
b))
127+
#".rkt"))))
45128
;; Tests with #f for directories
46129
(current-backup-dir #f)
47130
(current-autosave-dir #f)
@@ -96,6 +179,24 @@
96179
(equal? (simplify-path (generate-autosave-name complete))
97180
(simplify-path (generate-autosave-name clashing-name)))
98181
"files with the same name in different directories should not collide")
182+
; long path element
183+
(check-not-false
184+
(< (bytes-length (path-element->bytes
185+
(last (explode-path (generate-autosave-name
186+
too-long-pth)))))
187+
max-path-element-length)
188+
"excessively long elements should be shortened")
189+
; write permission
190+
(call-with-preference-not-writable
191+
current-autosave-dir
192+
(λ ()
193+
(test-case
194+
"autosave dir not writable"
195+
(check-false (memq 'write (file-or-directory-permissions autosave-dir))
196+
"autosave dir should have been made non-writable")
197+
(check-equal? (path-base (generate-autosave-name complete))
198+
dir-of-complete
199+
"should fall back when autosave dir not writable"))))
99200
; delete
100201
(delete-directory autosave-dir)
101202
(check-equal? (path-base (generate-autosave-name complete))
@@ -112,6 +213,24 @@
112213
(equal? (simplify-path (generate-backup-name complete))
113214
(simplify-path (generate-backup-name clashing-name)))
114215
"files with the same name in different directories should not collide")
216+
; long path element
217+
(check-not-false
218+
(< (bytes-length (path-element->bytes
219+
(last (explode-path (generate-backup-name
220+
too-long-pth)))))
221+
max-path-element-length)
222+
"excessively long elements should be shortened")
223+
; write permission
224+
(call-with-preference-not-writable
225+
current-backup-dir
226+
(λ ()
227+
(test-case
228+
"backup dir not writable"
229+
(check-false (memq 'write (file-or-directory-permissions backup-dir))
230+
"backup dir should have been made non-writable")
231+
(check-equal? (path-base (generate-backup-name complete))
232+
dir-of-complete
233+
"should fall back when backup dir not writable"))))
115234
; delete
116235
(delete-directory backup-dir)
117236
(check-equal? (path-base (generate-backup-name complete))

0 commit comments

Comments
 (0)