Title: A more intuitive fs.mkdtemp() · Issue #6142 · nodejs/node · GitHub
Open Graph Title: A more intuitive fs.mkdtemp() · Issue #6142 · nodejs/node
X Title: A more intuitive fs.mkdtemp() · Issue #6142 · nodejs/node
Description: Version: 5.10.1 Platform: Darwin 15.3.0 Subsystem: fs I am happy that the Node team decided to implement fs.mkdtemp() - the security concerns around this behavior make me glad to see it in core. However, the API feels awkward to me becau...
Open Graph Description: Version: 5.10.1 Platform: Darwin 15.3.0 Subsystem: fs I am happy that the Node team decided to implement fs.mkdtemp() - the security concerns around this behavior make me glad to see it in core. Ho...
X Description: Version: 5.10.1 Platform: Darwin 15.3.0 Subsystem: fs I am happy that the Node team decided to implement fs.mkdtemp() - the security concerns around this behavior make me glad to see it in core. Ho...
Opengraph URL: https://github.com/nodejs/node/issues/6142
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"A more intuitive fs.mkdtemp()","articleBody":"\u003c!--\r\nThanks for wanting to report an issue you've found in Node.js. Please fill in\r\nthe template below by replacing the html comments with an appropriate answer.\r\nIf unsure about something, just do as best as you're able.\r\n\r\nversion: `5.10.1`\r\nplatform: `Darwin 15.3.0`\r\nsubsystem: `fs`\r\n\r\nIt will be much easier for us to fix the issue if a test case that reproduces\r\nthe problem is provided. Ideally this test case should not have any external\r\ndependencies. We understand that it is not always possible to reduce your code\r\nto a small test case, but we would appreciate to have as\r\nmuch data as possible.\r\n\r\nThank you!\r\n--\u003e\r\n- **Version**: `5.10.1`\r\n- **Platform**: `Darwin 15.3.0`\r\n- **Subsystem**: `fs`\r\n\r\n\u003c!-- Enter your issue details below this comment. --\u003e\r\n\r\nI am happy that the Node team decided to implement `fs.mkdtemp()` - the security concerns around this behavior make me glad to see it in core.\r\n\r\nHowever, the API feels awkward to me because:\r\n- The randomness it provides is hard coded to 6 characters, which is just enough that it's _probably fine_, but not enough that I don't want to add more.\r\n- Using the `prefix` argument sounds like a good way to add random characters, but doing half the work is strange if you read code that actually does this. Additionally, _if_ I'm going to implement my own name pattern, I don't really want to _also_ have to keep in mind the one Node is using. For example, should I have to re-implement the entire `mkdtemp` just to get valid [UUIDv4](https://en.wikipedia.org/wiki/Universally_unique_identifier#Version_4_.28random.29) directory names? That is genuinely useful for unit tests and also for being able to easily `mv` the temporary directory to a cache-proof URL on my server without \"renaming\" it ... lest I have to undo the suffix every time or teach my client code about it.\r\n- Using `prefix` as a directory path is messy, What would you expect this code to do?\r\n \r\n ``` javascript\r\n 'use strict';\r\n \r\n const os = require('os');\r\n const fs = require('fs');\r\n \r\n fs.mkdtemp(os.tmpdir(), (err, dir) =\u003e {\r\n if (err) {\r\n throw err;\r\n }\r\n console.log('Created directory:', dir);\r\n });\r\n ```\r\n \r\n If `os.tmpdir()` returns `/tmp` for you, that will actually create a directory at the very root of your filesystem, rather than inside of `/tmp`. And it will end up being named something like `tmp-e0ew3m`. To \"fix\" this you have to use `path.join(os.tmpdir(), '/')`.\r\n\r\nI would like to propose making the API more intuitive and useful for its intended purpose: unique, convenient, secure creation of temporary directories.\r\n1. Have an explicit `cwd` argument, which is internally `path.join()`'d with the name. This is so that the name can be computed in isolation. And to prevent accidentally creating directories out in the open, where they won't actually be cleaned up.\r\n2. Replace `prefix` with (or just add) a `name` option, which is used as-is when provided. Possibly increase the default randomness for the case where one is not provided. This makes it easy to opt-out of the pattern that Node happens to use currently, which is not exposed via any kind of constant (and I don't think that would provide much value, anyway).\r\n\r\nI think these changes are best done as a breaking change, mainly to make the API less surprising. But compatibility could probably be maintained by only enabling these semantics on the options object.\r\n","author":{"url":"https://github.com/sholladay","@type":"Person","name":"sholladay"},"datePublished":"2016-04-11T04:45:47.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":4},"url":"https://github.com/6142/node/issues/6142"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:5e3caaed-7b4f-5c01-b3cd-11abcef9ee52 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | 9D5E:50919:71E9F6A:9FDD2D2:6A4FD4CD |
| html-safe-nonce | c5766c89d505cb09a6bfb448393d5052d759ab421a115f970f2426b64441e0f1 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI5RDVFOjUwOTE5OjcxRTlGNkE6OUZERDJEMjo2QTRGRDRDRCIsInZpc2l0b3JfaWQiOiI1MDAzODY2OTE4NTk3MTUzOTk3IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 9f5b427038051bf186ae63a705b31cabffcddb8d47bddfa390ffe18950dc3f22 |
| hovercard-subject-tag | issue:147315098 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/nodejs/node/6142/issue_layout |
| twitter:image | https://opengraph.githubassets.com/c225af67eb2f465ed0d173cb34aac976d08ae5b77710e1c47adc9dbdd99b3081/nodejs/node/issues/6142 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/c225af67eb2f465ed0d173cb34aac976d08ae5b77710e1c47adc9dbdd99b3081/nodejs/node/issues/6142 |
| og:image:alt | Version: 5.10.1 Platform: Darwin 15.3.0 Subsystem: fs I am happy that the Node team decided to implement fs.mkdtemp() - the security concerns around this behavior make me glad to see it in core. Ho... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | sholladay |
| hostname | github.com |
| expected-hostname | github.com |
| None | 19b98c3d9767bb8a56238a89ff8401d1cbdf3bc5c353799aacf49fe6c76b4b7c |
| turbo-cache-control | no-preview |
| go-import | github.com/nodejs/node git https://github.com/nodejs/node.git |
| octolytics-dimension-user_id | 9950313 |
| octolytics-dimension-user_login | nodejs |
| octolytics-dimension-repository_id | 27193779 |
| octolytics-dimension-repository_nwo | nodejs/node |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 27193779 |
| octolytics-dimension-repository_network_root_nwo | nodejs/node |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 142593e66413bbae27fd485859993597d5a3b104 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width