prusa3d / PrusaSlicer

G-code generator for 3D printers (RepRap, Makerbot, Ultimaker etc.)
https://www.prusa3d.com/prusaslicer/
GNU Affero General Public License v3.0
7.72k stars 1.93k forks source link

3mf path saving feature broken: stores _absolute_ paths instead of _relative_ paths to STLs #5540

Open hzeller opened 3 years ago

hzeller commented 3 years ago

Version

Version 2.3.0-rc1+linux-x64-GTK3

Behavior

Consider the following directory structure example; note how the stl file is stored in the fab/ subdirectory:

~/src/my/turntable$ ls -l output_gear.3mf fab/output_gear.stl 
-rw-r--r-- 1 hzeller hzeller 3510279 Dec 19 18:51 fab/output_gear.stl
-rw-r--r-- 1 hzeller hzeller  288649 Dec 20 10:37 output_gear.3mf

Now, let's look inside and see if the path is stored as a relative path to output_gear.3mf: fab/output_gear.stl

unzip output_gear.3mf Metadata/Slic3r_PE_model.config
grep fab/output_gear.stl Metadata/Slic3r_PE_model.config
   <metadata type="volume" key="source_file" value="/home/hzeller/src/my/turntable/fab/output_gear.stl"/>

NOO, it is absolute. It should contain only the relative path to the 3mf. In this case, the stl is in the fab sub-directory

 <metadata type="volume" key="source_file" value="fab/output_gear.stl"/>

Is this a new feature request?

More like a request to fix an otherwise great new feature before it goes out broken into the wild before 2.3 final :)

lukasmatena commented 3 years ago

The 2.3 version implements storing the stl filenames including paths.

It does not. This was implemented a year ago in 2.2. And in addition, citing from the 2.2.0-alpha4 change log:

AMF/3MF can now be configured to not save the full path to the source models, as this can pose a security risk when sharing such a file. If the full path is not saved and 'Reload from disk' is requested, PrusaSlicer will ask for the location of the source file. This option is now off by default (i.e. full paths are not saved).

It should be enough to go to Configuration->Preferences and uncheck Export sources full pathnames to 3mf and amf.

hzeller commented 3 years ago

Well without the option, only the filename is stored without any path, so in the example above, it would be output_gear.stl instead of fab/output_gear.stl.

So this is why I was excited to see the path to be included when I read the install wizzard screen.

However, the absolute path is indeed not useful (not to mention the security issue). And so is just the filename without any path.

So the only useful default option would be to store the relative path from the 3mf.

hzeller commented 3 years ago

Suggestion in the linked draft pull request.

Area5142 commented 3 years ago

I would like to see this relative path change too. I normally store STL files in sub directories to the project file to have a clean file structure.

bubnikv commented 3 years ago

I appreciate that @Area5142 liked that, I also tend to enjoy the emoticons, likes, hearts and +1ses.

However, I find the proposal quite non-transparent and if one is not aware of the behavior, she/he may reveal quite a significant portion of her/his disk paths to the referenced file. Indeed, what would you propose if the 3MF references a file on another disk (on Windows) or crossing the root (on Linux or OSX)? How many levels of reference would you allow this relative path to go?

Area5142 commented 3 years ago

@bubnikv I would suggest the following logic when referencing files from PS projects.

If Preferences "Export sources full path names to 3mf and amf" is unchecked:

If Preferences "Export sources full path names to 3mf and amf" is checked:

This is not perfect, but will satisfy most of the needs:

hzeller commented 3 years ago

I like @Area5142 's proposal. Initially, I was also considering sibling directories (so: ../foo), but just limiting strictly to current or subdirectories will fix the most common situation in which the stl's are in a subdirectory.

bubnikv commented 3 years ago

Given the extremely negative reaction to our introduction of hyperlinks on the parameter pages to our web knowledge base, I would be extremely cautious to introduce such a complex logic risking being dragged through the mud of social networks due to arguable privacy issues. Anything with the "privacy issue" sticker is extremely sensitive and we are not going to risk it to keep our mental health in check.

Thus if such a complex logic (sic! what sounds simple to you may sound incomprehensible to others) is to be implemented, we would need to add a confirmation dialog showing the path to be stored with possibly some UI to select from variants.

hzeller commented 3 years ago

But it is simple: the default with relative paths does exactly what everyone needs, and the absolute path choice is explained.

I do agree though that absolute path is not useful and should be removed, leaving only one option.

if two options are to be kept, make them 'no relative path, just filename' and 'relative path'. Drop the absolute option, as it only provides filesystem path name leakage.

bubnikv commented 3 years ago

But it is simple: the default with relative paths does exactly what everyone needs, and the absolute path choice is explained.

People do not read manuals, then they would distribute their 3MFs around and then they would drag us through the mud of social networks. While we likely have a couple of hundreds of thousands installs around the world, it takes just a couple of activists to generate a very negative reaction. Anything with the "security risk" tag is an invitation to those who have nothing better to do.

So we may implement your behavior, but only if the user is made explicitly aware of it by having to read and confirm, at least once. And people do not read, just click...

čt 7. 1. 2021 v 9:25 odesílatel Henner Zeller notifications@github.com napsal:

But it is simple: the default with relative paths does exactly what everyone needs, and the absolute path choice is explained.

I do agree though that absolute path is not useful and should be removed, leaving only one option.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/issues/5540#issuecomment-755964004, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMPSI3OSOBDXEJYJDF7HW3SYVVWDANCNFSM4VDJNVEQ .

hzeller commented 3 years ago

I am wondering why you are offering an absolute path in the first place, if this is a potential PR problem ?

The relative path reduces this problem. You still can do the same as now, where you explicitly choose the option to have it relative.

All I would like to have the 'absolute path' option change to actually do 'relative path' (with the same active choice by the user). Relative is much more useful and provides less issues with path leakage.

bubnikv commented 3 years ago

The absolute paths are disabled by default. This is a wizard step after I cleared the PrusaSlicer configuration directory and restarted from scratch. [image: image.png]

čt 7. 1. 2021 v 9:39 odesílatel Henner Zeller notifications@github.com napsal:

I am wondering why you are offering an absolute path in the first place, if this is a potential PR problem ?

The relative path reduces this problem. You still can do the same as now, where you explicitly choose the option to have it relative.

All I would like to have the 'absolute path' option change to actually do 'relative path' (with the same active choice by the user). Relative is much more useful and provides less issues with path leakage.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/issues/5540#issuecomment-755971005, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMPSI4CNWV333HQY3B5NLLSYVXMHANCNFSM4VDJNVEQ .

hzeller commented 3 years ago

Suggestion is to not even offer absolute paths as option. Just make the previous option for absolute paths store relative paths.

And you can use the same precaution to only enable this in the wizard.

Area5142 commented 3 years ago

I would be extremely cautious to introduce such a complex logic risking being dragged through the mud of social networks due to arguable privacy issues. Anything with the "privacy issue" sticker is extremely sensitive and we are not going to risk it to keep our mental health in check.

The two situations are very different and cannot be directly compared, from a privacy point of view. A direct link to the open Internet is a problem because it happens unexpectedly, launches an application program (browser) and downloads something that could be potentially harmful. At the same time, the browser sends a lot of information to the server about the user who performs the lookup - it is this information that is problematic.

A reference to a file (model) in a local PrusaSlicer project is not a security risk or privacy concern. Only when you actively choose to share your PS project by uploading it to others, you must, as always, be aware that it may contain information you do not want to share - such as knowledge of where files are located in the local file system, where the project was created and maybe copyright issues with the included models.

By saving relative paths instead of absolute ones, only the local project is exposed and therefore there are no privacy issues left. If you did not want to show the context of the local project, it should not have been uploaded in the first place.

I agree with @hzeller that the benefits of using relative references to files are great and make it possible to organize projects in a good way.

PS should automatically save references to files as relative paths when the file is located in the same directory as the project file or in a sub-directory.

If the use of "symlinks" (Mac and Linux) for directories and files is also allowed, any absolute references can be embedded in the project with symlinks without revealing their true location.

With the proposed changes there is no longer a need for a configuration option "Export sources full pathnames to 3mf and amf" and that can be removed.

bubnikv commented 3 years ago

I have a different proposal: Let's save the mapping from a saved 3MF to all the STLs it references in a single cache database in PrusaSlicer config directory. https://sqlite.org/ is perfect for that.

Area5142 commented 3 years ago

If location information to all STL/OBJ files is stored in a central database that PrusaSlicer manages for each 3MF project, it will probably create more problems than it solves.

There is a lot that can go wrong when renaming, moving and copying 3MF projects locally on the workstation or to a file server. This is often something I do to organize many projects or when I take a backup. It is now unclear which STL/OBJ files are now associated with a given 3MF project.

What happens when I access the same project from two different workstations (Linux and Windows) on a shared drive - they have different ways of referencing files on a shared drive.

I often run PrusaSlicer in the Alpha and Beta version with the same 3MF projects to test new features, do they each have their own database? What happens if I make a fresh reinstall of PrusaSlicer - is all my 3MF projects broken?

The mentioned problems are gone if 3MF project use relative references to STL/OBJ in the same directory and sub directories. The right model files will always be found even if the project is renamed, moved, copied or referenced by different PrusaSlicer versions on a file share.

Absolute paths could be created with symlinks (Linux) or shortcuts (Windows), but it will be an active choice and something that more advanced users will use.