Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Register
  • Sign in
  • erp5 erp5
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Merge requests 142
    • Merge requests 142
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Jobs
  • Commits
Collapse sidebar
  • nexedinexedi
  • erp5erp5
  • Merge requests
  • !1519

erp5_web_service: Do not change working directory on remote.

  • Review changes

  • Download
  • Patches
  • Plain diff
Closed Vincent Pelletier requested to merge vpelletier/erp5:sftp_relpath into master Dec 02, 2021
  • Overview 6
  • Commits 1
  • Pipelines 0
  • Changes 1

Allowing callers to exit the path set on the connector seem wrong to me, was this intended ? So I would rather always construct absolute paths locally (which allows checking them) than let callers accidentally write out of the path configured on the connector. The intent being to have fewer surprises when we change the connector's base path.

Also, this saves one round-trip on every connection (although this is likely unnoticeable for most uses).

To illustrate the intended behaviour changes (using writeFile as it is the most complex use-case):

connector url code before after
sftp://example.com/ writeFile(path='bar', filename='baz.txt') /bar/baz.txt /bar/baz.txt
sftp://example.com/foo writeFile(path='bar', filename='baz.txt') /foo/bar/baz.txt /foo/bar/baz.txt
sftp://example.com/ writeFile(path='/bar', filename='baz.txt') /bar/baz.txt /bar/baz.txt
sftp://example.com/foo writeFile(path='/bar', filename='baz.txt') /bar/baz.txt ⚠ /foo/bar/baz.txt
sftp://example.com/ writeFile(path='../bar', filename='baz.txt') /bar/baz.txt /bar/baz.txt1
sftp://example.com/foo writeFile(path='../bar', filename='baz.txt') /bar/baz.txt ⚠ ValueError
sftp://example.com/ writeFile(path='bar', filename='/baz.txt') /baz.txt /bar/baz.txt2
sftp://example.com/foo writeFile(path='bar', filename='/baz.txt') /baz.txt ⚠ /foo/bar/baz.txt2
sftp://example.com/ writeFile(path='bar', filename='../baz.txt') /baz.txt /baz.txt2
sftp://example.com/foo writeFile(path='bar', filename='../baz.txt') /foo/baz.txt /foo/baz.txt2
sftp://example.com/ writeFile(path='bar', filename='../../baz.txt') /baz.txt /baz.txt2
sftp://example.com/foo writeFile(path='bar', filename='../../baz.txt') /baz.txt ⚠ ValueError

@kazuhiko @jerome @georgios.dagkakis @aurel

  1. I would prefer to raise ValueError here, but the detection code would be quite complex just for this case... ↩

  2. The desirable behaviour in these cases is unclear to me... Is caller responsible of providing non-conflicting arguments ? ↩ ↩2 ↩3 ↩4 ↩5

Edited Dec 02, 2021 by Vincent Pelletier
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: sftp_relpath
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7