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

buf build xyz.tar fails if a file in this archive has an extended attribute #2387

Closed
oliversun9 opened this issue Aug 18, 2023 · 7 comments · Fixed by #2504
Closed

buf build xyz.tar fails if a file in this archive has an extended attribute #2387

oliversun9 opened this issue Aug 18, 2023 · 7 comments · Fixed by #2504
Labels
Bug Something isn't working

Comments

@oliversun9
Copy link
Contributor

oliversun9 commented Aug 18, 2023

Files created in vscode, and potentially other apps, have an extended attribute. Making a tape archive (.tar) and invoke buf build on such an archive will fail.

Steps to reproduce:
Run the following in the terminal built in to mac (not the one from vscode)

% mkdir dir1 dir2                              
% mv somewhere/vscode.proto dir1/vscode.proto #### assuming such a file exists and is created by vscode
% echo "message Foo {}" > dir1/vscode.proto    
% echo "message Bar {}" > dir2/terminal.proto  
% ls -al@ dir1/vscode.proto                    
-rw-r--r--@ 1 oliversun  staff  15 18 Aug 16:46 dir1/vscode.proto
	com.apple.provenance	11 
% ls -al@ dir2/terminal.proto                  
-rw-r--r--  1 oliversun  staff  15 18 Aug 16:46 dir2/terminal.proto
% tar -cvf t1.tar dir1                         
a dir1
a dir1/vscode.proto
% tar -cvf t2.tar dir2                         
a dir2
a dir2/terminal.proto
% buf --version                                
1.22.0
% buf build t2.tar 
% buf build t1.tar
dir1/._vscode.proto:1:1:invalid control character
dir1/._vscode.proto:1:1:syntax error: unexpected error
dir1/._vscode.proto:1:2:invalid control character
...

Verified on 1.22.0 and 1.26.2-dev (latest main).

@oliversun9 oliversun9 added the Bug Something isn't working label Aug 18, 2023
@oliversun9 oliversun9 changed the title buf build xyz.tar if a file in this archive has an extended attribute buf build xyz.tar fails if a file in this archive has an extended attribute Aug 18, 2023
@bufdev
Copy link
Member

bufdev commented Aug 18, 2023

Can you see if buf build dir1/vscode.proto fails as well?

@oliversun9
Copy link
Contributor Author

Can you see if buf build dir1/vscode.proto fails as well?

Both buf build dir1/vscode.proto and buf build dir2/terminal.proto succeed.

@oliversun9
Copy link
Contributor Author

oliversun9 commented Aug 18, 2023

Some more information:

adding a print statement in storageArchive.Untar shows that during buf build t1.tar, the following files are looked at:

dir1/
dir1/._vscode.proto
dir1/vscode.proto

and during buf build t2.tar, it looked at these files:

dir2/
dir2/terminal.proto

This isn't surprising, given we have seen the error messages: dir1/._vscode.proto:1:1:invalid control character.

It turns out that the ._ files are used by the Mac to store extended attributes.

Some options:

  1. Skip ._ files in a tar whenever we see one
  2. Maintain the status quo
  3. Skip xyz/._a.proto if xyz/a.proto is also present.

@timostamm
Copy link
Member

The macos Archive Utility.app does something similar when creating ZIP archives:

$ unzip -l proto.zip 
Archive:  proto.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  10-17-2023 17:50   proto/
        4  10-17-2023 17:50   proto/x.proto
      327  10-17-2023 17:50   __MACOSX/proto/._x.proto
---------                     -------
      331                     3 files

jchadwick-buf added a commit that referenced this issue Oct 17, 2023
This commit introduces a workaround for an issue with TAR archives
created with libarchive on Darwin/macOS. When encountering a file that
has extended attributes, such as the com.apple.provenance attribute, it
will add a corresponding AppleDouble file with the prefix "._" in the
same directory. Because these files have the same filename otherwise,
the buf tool will see these when they correspond to proto files and
subsequently fail to parse them as protobuf IDL.

Due to the fact that libarchive is used by default with the version of
tar that ships with macOS, and the provenance extended attribute is set
by macOS under many conditions when SIP is enabled, archives with these
files are likely to occur on macOS.

This workaround is not needed by .zip as libarchive .zip files do not
seem to have the same behavior.

Closes #2387.
@jchadwick-buf
Copy link
Member

jchadwick-buf commented Oct 17, 2023

Hmm, that's interesting. I didn't catch that behavior with libarchive ZIP, unless I was missing an option. Maybe it should be a separate issue...

edit: Though, the same workaround works for both, so it's tempting to just make this behavior happen for all archives.

@oliversun9
Copy link
Contributor Author

I remember making both a .zip and a .tar back then. The .zip archive worked but not the .tar.

@timostamm
Copy link
Member

The macos Archive Utility.app creates these files. To reproduce, right click a directory with proto files in the Finder, select "Compress", and run buf build on the resulting zip file to see a long list of errors such as __MACOSX/proto/._x.proto:1:1:invalid control character.

Though, the same workaround works for both, so it's tempting to just make this behavior happen for all archives.

That's my thinking as well, and why I commented 🙂

jchadwick-buf added a commit that referenced this issue Oct 17, 2023
This commit introduces a workaround for an issue with .tar archives
created with libarchive on Darwin/macOS. When encountering a file that
has extended attributes, such as the com.apple.provenance attribute, it
will add a corresponding AppleDouble file with the prefix "._" in the
same directory. Because these files have the same filename otherwise,
the buf tool will see these when they correspond to proto files and
subsequently fail to parse them as protobuf IDL.

Due to the fact that libarchive is used by default with the version of
tar that ships with macOS, and the provenance extended attribute is set
by macOS under many conditions when SIP is enabled, archives with these
files are likely to occur on macOS.

.zip archives created by libarchive do not seem to have this issue.
However, .zip archives created by Archive Utility.app have a very
similar behavior when encountering extended attributes: it will place
AppleDouble files in a separate directory tree under the MACOSX
directory, but also with the "._" prefix. Since this can be handled by
the same logic, this behavior is extended to ZIP files as well.

Closes #2387.
jchadwick-buf added a commit that referenced this issue Oct 18, 2023
This commit introduces a workaround for an issue with .tar archives
created with libarchive on Darwin/macOS. When encountering a file that
has extended attributes, such as the com.apple.provenance attribute, it
will add a corresponding AppleDouble file with the prefix "._" in the
same directory. Because these files have the same filename otherwise,
the buf tool will see these when they correspond to proto files and
subsequently fail to parse them as protobuf IDL.

Due to the fact that libarchive is used by default with the version of
tar that ships with macOS, and the provenance extended attribute is set
by macOS under many conditions when SIP is enabled, archives with these
files are likely to occur on macOS.

.zip archives created by libarchive do not seem to have this issue.
However, .zip archives created by Archive Utility.app have a very
similar behavior when encountering extended attributes: it will place
AppleDouble files in a separate directory tree under the MACOSX
directory, but also with the "._" prefix. Since this can be handled by
the same logic, this behavior is extended to ZIP files as well.

Closes #2387.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants