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

Alternate data streams are removed by saving #6363

Closed
NotepadPlusUser opened this issue May 14, 2016 · 10 comments

Comments

@NotepadPlusUser
Copy link

commented May 14, 2016

  • VSCode Version:1.1.0
  • OS Windows Pro 10.0.10586.318 V1511

Steps to Reproduce:

  1. Open with VSCode a text file that has one or more alternate data stream files.
  2. Make some small change in the text.
  3. Save the file.
  4. The ADS files have been removed.

I'm sure that there is a setting that will prevent VSCode removing ADS files when saving — they are, after all, a Windows special. If not, could you please regard it as a bug and add such an option.

@bpasero

This comment has been minimized.

Copy link
Member

commented May 26, 2016

@NotepadPlusUser I would expect this issue to be there in standalone node.js already, if you could try out a fs.writeFile() to reproduce the issue you should report it to them.

@bpasero bpasero added this to the Backlog milestone May 26, 2016

@NotepadPlusUser

This comment has been minimized.

Copy link
Author

commented May 27, 2016

Thanks, bpasero, for looking at this issue and adding the 'bug' stamp to it. I have to apologise, however, that I don't understand what your reply means.

  1. Is your reply directed to me, or to the developers of VSCode?
  2. If the reply is to me, and is asking me to do something:
    (a) What does 'standalone node.js' mean? (I have a rather basic knowledgeof JScript.)
    (b) What is a 'fs.writefile()'?
    (c) Who is 'them?
    (d) What more information is needed? My 'steps to reproduce' seem a complete description of the issue and how to reproduce it.
@bpasero

This comment has been minimized.

Copy link
Member

commented May 27, 2016

@NotepadPlusUser node.js is the framework we use for writing to files so it would be interesting if this bug is a bug in node.js (https://nodejs.org/en/). By writing a small script like the following we could see if the issue is node.js related or not (using fs.writeFileSync from https://nodejs.org/docs/v5.10.0/api/fs.html#fs_fs_writefilesync_file_data_options):

var fs = require("fs");

fs. writeFileSync(filepath, "data");

Where filepath is the path to the file with data streams.

@NotepadPlusUser

This comment has been minimized.

Copy link
Author

commented May 27, 2016

Apologies bpasero, I'm completely out of my depth here.

I tried running the two lines as a bare .js file, but I get the error: Line 1, Char 1, Object expected. I tried running it another environment with the same result, By the way, in line 2, I had substituted filepath with the full path of the target file:
"D:\DataForVallaHome\Scripts\WSCLibrary\TestADSFiles.txt"

I tried instead to post you a TXT file that has two ADS TXT files attached, but of course the transfer to your servers removed them (I tested that by previewing and re-downloading).

Then I tried zipping the file first, but that also removed the ADS files..

So I'll have to leave it to the developers to sort out the origin of the problem and the fix. It's a serious matter for windows users, because you often want to put file metadata in an ADS file.

@bpasero

This comment has been minimized.

Copy link
Member

commented May 27, 2016

@NotepadPlusUser can you maybe explain how one gets a file with data streams on Windows? Then I can try to reproduce in node.js.

@NotepadPlusUser

This comment has been minimized.

Copy link
Author

commented May 28, 2016

[Next morning] Sure. Using good old DOS is the most direct way. The following 5 lines of DOS code will:

  1. Create a file called "MainFile.txt" in the current directory
  2. Create two attached ADS files, "FirstADSFile.txt" and "SecondADSFile.txt"
  3. List the current directory — the /r switch is what displays the otherwise hidden ADS files.

Echo "This is the main file" > MainFile.txt
Echo "Some content for the first ADS file" > MainFile.txt:FirstADSFile.txt
Echo "Some content for the second ADS file" > MainFile.txt:SecondADSFile.txt
Dir /r
Pause


So to reproduce the problem:

  1. Run these five lines from a DOS batch file or in a DOS window (preferably working within a directory with few files in it), and you will see the two attached ADS files listed on separate lines under MainFile.txt.
  2. Then open MainFile.txt with VSCode, make a small change to the text, and save it.
  3. Then run just the last two lines of the DOS code above, and the Dir /r command will now show that the two ADS files have been removed. I want to prevent that removal.
@NotepadPlusUser

This comment has been minimized.

Copy link
Author

commented May 29, 2016

Thanks very much, bpasero, for handling this issue and confirming it. I'll keep a watch on the updates as they come out, and report back.

@bpasero

This comment has been minimized.

Copy link
Member

commented May 30, 2016

Thanks I am able to reproduce with just node.js so this is indeed an upstream issue it seems.

@f0zi

This comment has been minimized.

Copy link

commented Feb 3, 2018

All right, as @seishun mentioned:

To preserve data in alternate streams, consider using the 'r+' flag and ftruncate instead

@seishun

This comment has been minimized.

Copy link
Contributor

commented Feb 3, 2018

VS Code already uses this logic for hidden files thanks to #31733. So it just needs to be applied to all existing files.

@bpasero bpasero added bug and removed upstream labels Aug 20, 2018

@bpasero bpasero added this to the August 2018 milestone Aug 20, 2018

bpasero added a commit that referenced this issue Aug 20, 2018

Use 'r+' with truncation when saving existing files on Windows (#42899)
* Use 'r+' with truncation when saving existing files on Windows

Opening a file with 'w' flag removes its alternate data streams. To
prevent this, extend the logic used for hidden files to all existing
files.

Fixes: #6363

* adopt truncate for saving as admin

@chrmarti chrmarti added the verified label Aug 30, 2018

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.