Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use is_file_path_valid instead of is_safe_path #18316

Merged
merged 2 commits into from Oct 28, 2022

Conversation

sagarvora
Copy link
Collaborator

@sagarvora sagarvora commented Oct 7, 2022

Alternative to #18313

Support symlinked public/private file directories.

@sagarvora sagarvora requested review from surajshetty3416 and a team as code owners October 7, 2022 07:24
@sagarvora sagarvora requested review from phot0n and ankush and removed request for a team and phot0n October 7, 2022 07:24
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Oct 7, 2022
@sagarvora sagarvora removed the request for review from surajshetty3416 October 7, 2022 07:24
@sagarvora sagarvora changed the title fix: use validate_file_path instead of is_safe_path fix: use is_file_path_valid instead of is_safe_path Oct 7, 2022
@tukutela
Copy link

Hi all, I'm the creator of the the PR #18313 and I'm wondering when this PR will be merged into version-14. I've got some workarounds going at the moment but I'd rather keep a clean install running for my client.

@sagarvora
Copy link
Collaborator Author

@tukutela I will try to get this merged this week. Thanks for your patience.

@ankush ankush self-assigned this Oct 12, 2022
@stale stale bot added the inactive label Oct 20, 2022
@frappe frappe deleted a comment from stale bot Oct 20, 2022
@stale stale bot removed the inactive label Oct 20, 2022
@tukutela
Copy link

Wondering if I should re-submit my PR, this solution is better though.

@sagarvora
Copy link
Collaborator Author

@tukutela PR is ready.

Note for reviewer: This PR attempts to refactor as little as possible, focusing on fixing the current issue instead. A minor refactor has been attempted in #18584.

@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #18316 (52e9a12) into develop (5c2b917) will increase coverage by 0.00%.
The diff coverage is 85.71%.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #18316    +/-   ##
=========================================
  Coverage    63.16%   63.17%            
=========================================
  Files          747      747            
  Lines        67248    67400   +152     
  Branches      5995     5995            
=========================================
+ Hits         42477    42577   +100     
- Misses       21312    21364    +52     
  Partials      3459     3459            
Flag Coverage Δ
server-mariadb 67.17% <90.90%> (-0.03%) ⬇️
server-postgres 67.18% <90.90%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@sagarvora sagarvora removed dont-merge add-test-cases Add test case to validate fix or enhancement labels Oct 27, 2022
@ankush ankush merged commit d35aa6c into frappe:develop Oct 28, 2022
@ankush ankush deleted the fix-always-validate branch October 28, 2022 08:26
@ankush ankush added the backport version-14-hotfix backport to version 14 label Oct 28, 2022
mergify bot pushed a commit that referenced this pull request Oct 28, 2022
* fix: use `validate_file_path` instead of `is_safe_path`

* test: specify `is_private` for file with private URL

(cherry picked from commit d35aa6c)
ankush pushed a commit that referenced this pull request Oct 28, 2022
)

* fix: use `validate_file_path` instead of `is_safe_path`

* test: specify `is_private` for file with private URL

(cherry picked from commit d35aa6c)

Co-authored-by: Sagar Vora <sagar@resilient.tech>
ankush added a commit that referenced this pull request Oct 31, 2022
ankush added a commit that referenced this pull request Nov 1, 2022
ankush added a commit that referenced this pull request Nov 1, 2022
ankush added a commit that referenced this pull request Nov 1, 2022
frappe-pr-bot pushed a commit that referenced this pull request Nov 1, 2022
# [14.14.0](v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([#18633](#18633)) ([#18634](#18634)) ([17821f1](17821f1))
* **formatting:** use snake case for variable names ([32851ce](32851ce))
* handle exceptions thrown in post_schema_updates ([#18648](#18648)) ([#18663](#18663)) ([29a659a](29a659a))
* **insert_many:** list instead of set to maintain order ([#18641](#18641)) ([4c521b8](4c521b8))
* make module def custom checkbox readonly in prod mode ([#18698](#18698)) ([18fa732](18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](1b70ec4))
* set proper cache key for singles when name is passed as `None` ([#18667](#18667)) ([#18668](#18668)) ([e32ba96](e32ba96))
* Signature form ([#18477](#18477)) ([8e4711b](8e4711b))
* Signature form (backport [#18477](#18477)) ([#18690](#18690)) ([34c1611](34c1611))
* support symlinked /files directory ([#18702](#18702)) ([7b8cbd0](7b8cbd0))
* use `async...await` when parsing route ([9c09c65](9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([#18316](#18316)) ([#18642](#18642)) ([537966c](537966c))
* **UX:** Info message on workspace for better UX (backport [#18701](#18701)) ([#18703](#18703)) ([5ba85ec](5ba85ec))
* **UX:** session expiry explanation and defaults ([#18680](#18680)) ([#18685](#18685)) ([515f6c2](515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([#18587](#18587)) ([#18588](#18588)) ([c8728f5](c8728f5))
* webform validation script not working ([ab2b8df](ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [#17851](#17851)) ([#18456](#18456)) ([b04a54c](b04a54c))
* support list view for "show titles instead of name" ([#18060](#18060)) ([#18681](#18681)) ([64c2555](64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([#18665](#18665)) ([#18669](#18669)) ([5f7928b](5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (#18316) (#18642)" (#18696) ([a08c029](a08c029)), closes [#18316](#18316) [#18642](#18642) [#18696](#18696)
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
frappe#18642)

* fix: use `validate_file_path` instead of `is_safe_path`

* test: specify `is_private` for file with private URL

(cherry picked from commit d35aa6c)

Co-authored-by: Sagar Vora <sagar@resilient.tech>
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
# [14.14.0](frappe/frappe@v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([frappe#18633](frappe#18633)) ([frappe#18634](frappe#18634)) ([17821f1](frappe@17821f1))
* **formatting:** use snake case for variable names ([32851ce](frappe@32851ce))
* handle exceptions thrown in post_schema_updates ([frappe#18648](frappe#18648)) ([frappe#18663](frappe#18663)) ([29a659a](frappe@29a659a))
* **insert_many:** list instead of set to maintain order ([frappe#18641](frappe#18641)) ([4c521b8](frappe@4c521b8))
* make module def custom checkbox readonly in prod mode ([frappe#18698](frappe#18698)) ([18fa732](frappe@18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](frappe@1b70ec4))
* set proper cache key for singles when name is passed as `None` ([frappe#18667](frappe#18667)) ([frappe#18668](frappe#18668)) ([e32ba96](frappe@e32ba96))
* Signature form ([frappe#18477](frappe#18477)) ([8e4711b](frappe@8e4711b))
* Signature form (backport [frappe#18477](frappe#18477)) ([frappe#18690](frappe#18690)) ([34c1611](frappe@34c1611))
* support symlinked /files directory ([frappe#18702](frappe#18702)) ([7b8cbd0](frappe@7b8cbd0))
* use `async...await` when parsing route ([9c09c65](frappe@9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([frappe#18316](frappe#18316)) ([frappe#18642](frappe#18642)) ([537966c](frappe@537966c))
* **UX:** Info message on workspace for better UX (backport [frappe#18701](frappe#18701)) ([frappe#18703](frappe#18703)) ([5ba85ec](frappe@5ba85ec))
* **UX:** session expiry explanation and defaults ([frappe#18680](frappe#18680)) ([frappe#18685](frappe#18685)) ([515f6c2](frappe@515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([frappe#18587](frappe#18587)) ([frappe#18588](frappe#18588)) ([c8728f5](frappe@c8728f5))
* webform validation script not working ([ab2b8df](frappe@ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [frappe#17851](frappe#17851)) ([frappe#18456](frappe#18456)) ([b04a54c](frappe@b04a54c))
* support list view for "show titles instead of name" ([frappe#18060](frappe#18060)) ([frappe#18681](frappe#18681)) ([64c2555](frappe@64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([frappe#18665](frappe#18665)) ([frappe#18669](frappe#18669)) ([5f7928b](frappe@5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (frappe#18316) (frappe#18642)" (frappe#18696) ([a08c029](frappe@a08c029)), closes [frappe#18316](frappe#18316) [frappe#18642](frappe#18642) [frappe#18696](frappe#18696)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants