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

Creation of sketches with non-compliant names is permitted #1599

Closed
3 tasks done
per1234 opened this issue Oct 29, 2022 · 5 comments · Fixed by #1833
Closed
3 tasks done

Creation of sketches with non-compliant names is permitted #1599

per1234 opened this issue Oct 29, 2022 · 5 comments · Fixed by #1833
Assignees
Labels
conclusion: resolved Issue was resolved criticality: medium Of moderate impact topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@per1234
Copy link
Contributor

per1234 commented Oct 29, 2022

Describe the problem

The Arduino sketch specification defines the characters allowed for use in sketch folder and file names:

https://arduino.github.io/arduino-cli/dev/sketch-specification/#sketch-folders-and-files

🐛 Arduino IDE allows users to create sketches with non-compliant names

To reproduce

  1. Select File > New from the Arduino IDE menus.
  2. Select File > Save As... from the Arduino IDE menus.
  3. Save the sketch under the sketchbook folder, with the name `~!@#$%^&()=+[{]};',
    🐛 The IDE allowed the creation of a sketch with non-compliant name.
  4. Click the ●●● icon on the right side of the editor toolbar.
  5. Select "New Tab" from the menu.
  6. In the "Name for new file" field, type `~!@#$%^&()=+[{]};',2
  7. Click the OK button.
    🐛 The IDE allowed the creation of an additional sketch file with non-compliant name.
  8. Select Tools > Board > Arduino AVR Boards > Arduino Uno from the Arduino IDE menus.
  9. Select Sketch > Verify/Compile from the Arduino IDE menus.
    🐛 Compilation fails with a cryptic error:
    avr-g++: error: C:\Users\per\AppData\Local\Temp\arduino-sketch-DC0B853C212DB0BA0ED269B37923BDC7\sketch\`~!@#$%^&()=+[;',.ino.cpp: No such file or directory
    avr-g++: warning: '-x c++' after last input file has no effect
    avr-g++: fatal error: no input files
    compilation terminated.
    
    exit status 1
    
    Compilation error: exit status 1
    
  10. Start Arduino IDE 1.x
    🐛 A warning dialog is shown on startup:
    image
    🐛 The sketch can not be shared with Arduino IDE 1.x users due to the non-compliant name.
    This problem seems less significant in this contrived demo where the sketch name causes compilation failure, but other non-compliant sketch names will not have an obvious impact to the Arduino IDE 2.x user.
  11. Import the sketch to Arduino Web Editor
    ⚠ Due to a bug in Arduino Web Editor, the sketch will be impossible to delete and might possibly cause other problems, so I don't recommend actually doing this with any Arduino account you care about.
    🐛 There is an error message on import.
  12. Delete the imported sketch from your Arduino Web Editor sketchbook.
    🐛 The sketch is impossible to delete:

    Error while deleting `~!@#$%^&()=+[{]};',

🐛 Arduino IDE 2.x allows the creation of sketches that are either unusable or not portable to other official development software.

Expected behavior

Arduino IDE 2.x does not allow the creation of sketches with non-compliant names.

Sketches created with one development tool are usable in all other tools.

Arduino IDE version

2.0.1

Operating system

  • Windows
  • Linux

Operating system version

  • Windows 10
  • Ubuntu 20.04

Additional context

Originally reported by @UKHeliBob at https://forum.arduino.cc/t/file-save-as-allows-invalid-filenames-to-be-used/700966


Related:

Issue checklist

  • I searched for previous reports in the issue tracker
  • I verified the problem still occurs when using the latest nightly build
  • My report contains all necessary details
@per1234 per1234 added criticality: medium Of moderate impact topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Oct 29, 2022
@kittaakos
Copy link
Contributor

kittaakos commented Jan 16, 2023

I have collected the differences between the spec and how it works in the create editor https://create.arduino.cc/editor:

Allowed to start with:

  • spec -> [0-9a-zA-Z]
  • create -> [0-9a-zA-Z_]

Allowed characters:

  • spec -> [0-9a-zA-Z_\.-]
  • create -> [0-9a-zA-Z_]

Allowed length:

  • spec -> {1-63}
  • create -> {1-36}

The problem is that the Create API is more restrictive than the spec. Hence, it won't be possible in IDE2 to constantly push any local sketch to the cloud due to the invalid name. To summarize, what is valid and works as a local sketch might not be OK as a remote sketch. I need to know how this was designed to handle in IDE2. Thanks!

@kittaakos
Copy link
Contributor

kittaakos commented Jan 16, 2023

As a proposal, IDE2 can use the spec rules when creating ordinary sketches, and when the user wants to push them, and the name of the sketch does not comply with the Create API naming convention, users must rename it before a push. What do you think, @91volt?

Update:

Yes. IDE2 has to raise a dialog before the push, and the local sketch will be pushed with a different name:

Screen Shot 2023-01-17 at 10 21 54

@kittaakos kittaakos added the status: in progress Work is in progress on this label Jan 17, 2023
@kittaakos
Copy link
Contributor

Sketches created with one development tool are usable in all other tools.

I do not think this is always feasible. Example: IDE2 has a valid sketch name, export the sketch to ZIP, still valid, and import it to Create editor, it could be invalid.

kittaakos pushed a commit that referenced this issue Jan 17, 2023
@per1234
Copy link
Contributor Author

per1234 commented Jan 18, 2023

It is absolutely feasible. The Arduino Cloud developers just need to respect the Arduino Sketch Specification.

@kittaakos
Copy link
Contributor

kittaakos commented Jan 24, 2023

Expected behavior

IDE2 has to validate the sketch code files besides the main sketch file and the sketch folder. Validation of any new file is trivial; the question was when to validate the sketch code files of existing sketches, how to warn users, and how to guide them to fix the issues. Per and I further refined the requirements of this task and decided to do the following:

  • IDE2 will show any sketches under File > Sketchbook if the sketch folder name matches the main sketch file name without the extension. This is sufficient for IDE2 to ask the CLI to load the sketch.
  • Unlike IDE 1.x, IDE2 won't validate other sketches in the sketchbook on app start, only the sketch being opened.
  • IDE2 will propose a series of actions to fix the problem when a user opens an invalid sketch. IDE2 can prompt the following actions. The order of the actions matter:
    • Rename the sketch folder and the main sketch file if invalid (this should be handled with a Save as..., after fixing the folder and main sketch file name, IDE2 deletes the previous sketch and reloads the renamed sketch in the same window,
    • The same validation logic runs. The sketch folder name must be valid at this point. IDE2 will prompt actions to fix any code file naming issues (IDE2 will use the new file, rename file dialog)
    • Any time the user interrupts the series sketch folder or code file renaming, IDE2 will interrupt the sketch opening and opens a new temp sketch in the same window.
    • The series of proposed actions happen in a modal fashion. IDE2 does not allow users to access the UI (compile or upload sketch, change boards, etc.) until the sketch is valid.

For further details on this topic, please reference #1821 (review).

kittaakos pushed a commit that referenced this issue Jan 24, 2023
Closes #1599

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Jan 25, 2023
Closes #1599

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Jan 31, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 1, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 1, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 1, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 2, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 2, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 2, 2023
Closes #1599
Closes #1825
Closes #1826

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 2, 2023
Closes #1599
Closes #1825
Closes #1826
Closes #1847

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 2, 2023
Closes #1599
Closes #1825
Closes #1826
Closes #1847

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 3, 2023
Closes #1599
Closes #1825
Closes #1826
Closes #1847

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 8, 2023
Closes #1599
Closes #1825
Closes #1826
Closes #1847

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 13, 2023
Closes #1599
Closes #1825
Closes #1826
Closes #1847

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 13, 2023
Closes #1599
Closes #1825
Closes #649
Closes #1847
Closes #1882

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
kittaakos pushed a commit that referenced this issue Feb 15, 2023
Closes #1599
Closes #1825
Closes #649
Closes #1847
Closes #1882

Co-authored-by: Akos Kitta <[email protected]>
Co-authored-by: per1234 <[email protected]>

Signed-off-by: Akos Kitta <[email protected]>
@per1234 per1234 added the conclusion: resolved Issue was resolved label Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved criticality: medium Of moderate impact topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
4 participants