Skip to content

Fix library verification when installing from git repo or zip file #1466

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

Merged
merged 2 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions arduino/globals/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,11 @@ var (
".cpp": empty,
".S": empty,
}

// HeaderFilesValidExtensions lists valid extensions for header files
HeaderFilesValidExtensions = map[string]struct{}{
".h": empty,
".hpp": empty,
".hh": empty,
}
)
7 changes: 6 additions & 1 deletion arduino/libraries/libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
paths "github.com/arduino/go-paths-helper"
Expand Down Expand Up @@ -233,7 +234,11 @@ func (library *Library) SourceHeaders() ([]string, error) {
if err != nil {
return nil, fmt.Errorf(tr("reading lib src dir: %s"), err)
}
cppHeaders.FilterSuffix(".h", ".hpp", ".hh")
headerExtensions := []string{}
for k := range globals.HeaderFilesValidExtensions {
headerExtensions = append(headerExtensions, k)
}
cppHeaders.FilterSuffix(headerExtensions...)
res := []string{}
for _, cppHeader := range cppHeaders {
res = append(res, cppHeader.Base())
Expand Down
58 changes: 42 additions & 16 deletions arduino/libraries/librariesmanager/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"strings"

"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/arduino/libraries"
"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
"github.com/arduino/arduino-cli/arduino/utils"
Expand Down Expand Up @@ -139,7 +140,7 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin
extractionPath := paths[0]
libraryName := extractionPath.Base()

if err := validateLibrary(libraryName, extractionPath); err != nil {
if err := validateLibrary(extractionPath); err != nil {
return err
}

Expand Down Expand Up @@ -229,7 +230,7 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
return err
}

if err := validateLibrary(libraryName, installPath); err != nil {
if err := validateLibrary(installPath); err != nil {
// Clean up installation directory since this is not a valid library
installPath.RemoveAll()
return err
Expand Down Expand Up @@ -259,22 +260,47 @@ func parseGitURL(gitURL string) (string, error) {
return res, nil
}

// validateLibrary verifies the dir contains a valid library, meaning it has both
// an header <name>.h, either in src or root folder, and a library.properties file
func validateLibrary(name string, dir *paths.Path) error {
// Verify library contains library header.
// Checks also root folder for legacy reasons.
// For more info see:
// https://arduino.github.io/arduino-cli/latest/library-specification/#source-code
libraryHeader := name + ".h"
if !dir.Join("src", libraryHeader).Exist() && !dir.Join(libraryHeader).Exist() {
return fmt.Errorf(tr(`library is not valid: missing header file "%s"`), libraryHeader)
// validateLibrary verifies the dir contains a valid library, meaning it has either
// library.properties file and an header in src/ or an header in its root folder.
// Returns nil if dir contains a valid library, error on all other cases.
func validateLibrary(dir *paths.Path) error {
if dir.NotExist() {
return fmt.Errorf(tr("directory doesn't exist: %s", dir))
}

// Verifies library contains library.properties
if !dir.Join("library.properties").Exist() {
return fmt.Errorf(tr(`library is not valid: missing file "library.properties"`))
searchHeaderFile := func(d *paths.Path) (bool, error) {
if d.NotExist() {
// A directory that doesn't exist can't obviously contain any header file
return false, nil
}
dirContent, err := d.ReadDir()
if err != nil {
return false, fmt.Errorf(tr("reading directory %s content: %w", dir, err))
}
dirContent.FilterOutDirs()
headerExtensions := []string{}
for k := range globals.HeaderFilesValidExtensions {
headerExtensions = append(headerExtensions, k)
}
dirContent.FilterSuffix(headerExtensions...)
return len(dirContent) > 0, nil
}

return nil
// Library format rev2
// https://arduino.github.io/arduino-cli/latest/library-specification/#15-library-format-rev-22
if headerFound, err := searchHeaderFile(dir.Join("src")); err != nil {
return err
} else if dir.Join("library.properties").Exist() && headerFound {
return nil
}

// Legacy library format
// https://arduino.github.io/arduino-cli/latest/library-specification/#old-library-format-pre-15
if headerFound, err := searchHeaderFile(dir); err != nil {
return err
} else if headerFound {
return nil
}

return fmt.Errorf(tr("library not valid"))
}
41 changes: 41 additions & 0 deletions arduino/libraries/librariesmanager/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package librariesmanager
import (
"testing"

"github.com/arduino/go-paths-helper"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -62,3 +63,43 @@ func TestParseGitURL(t *testing.T) {
require.Equal(t, "arduino-lib", libraryName)
require.NoError(t, err)
}

func TestValidateLibrary(t *testing.T) {
tmpDir := paths.New(t.TempDir())

nonExistingDirLib := tmpDir.Join("nonExistingDirLib")
err := validateLibrary(nonExistingDirLib)
require.Errorf(t, err, "directory doesn't exist: %s", nonExistingDirLib)

emptyLib := tmpDir.Join("emptyLib")
emptyLib.Mkdir()
err = validateLibrary(emptyLib)
require.Errorf(t, err, "library not valid")

onlyPropertiesLib := tmpDir.Join("onlyPropertiesLib")
onlyPropertiesLib.Mkdir()
onlyPropertiesLib.Join("library.properties").WriteFile([]byte{})
err = validateLibrary(onlyPropertiesLib)
require.Errorf(t, err, "library not valid")

onlyHeaderLib := tmpDir.Join("onlyHeaderLib")
onlyHeaderLibSourceDir := onlyHeaderLib.Join("src")
onlyHeaderLibSourceDir.MkdirAll()
onlyHeaderLibSourceDir.Join("some_file.hpp").WriteFile([]byte{})
err = validateLibrary(onlyHeaderLib)
require.Errorf(t, err, "library not valid")

validLib := tmpDir.Join("valiLib")
validLibSourceDir := validLib.Join("src")
validLibSourceDir.MkdirAll()
validLibSourceDir.Join("some_file.hpp").WriteFile([]byte{})
validLib.Join("library.properties").WriteFile([]byte{})
err = validateLibrary(validLib)
require.NoError(t, err)

validLegacyLib := tmpDir.Join("validLegacyLib")
validLegacyLib.Mkdir()
validLegacyLib.Join("some_file.hpp").WriteFile([]byte{})
err = validateLibrary(validLib)
require.NoError(t, err)
}
50 changes: 27 additions & 23 deletions i18n/data/en.po
Original file line number Diff line number Diff line change
Expand Up @@ -2238,10 +2238,10 @@ msgstr "Use the specified programmer to upload."
msgid "Used: {0}"
msgstr "Used: {0}"

#: arduino/libraries/librariesmanager/install.go:67
#: arduino/libraries/librariesmanager/install.go:83
#: arduino/libraries/librariesmanager/install.go:105
#: arduino/libraries/librariesmanager/install.go:189
#: arduino/libraries/librariesmanager/install.go:68
#: arduino/libraries/librariesmanager/install.go:84
#: arduino/libraries/librariesmanager/install.go:106
#: arduino/libraries/librariesmanager/install.go:190
msgid "User directory not set"
msgstr "User directory not set"

Expand Down Expand Up @@ -2355,7 +2355,7 @@ msgstr "Writing config file: %v"
msgid "archive hash differs from hash in index"
msgstr "archive hash differs from hash in index"

#: arduino/libraries/librariesmanager/install.go:136
#: arduino/libraries/librariesmanager/install.go:137
msgid "archive is not valid: multiple files found in zip file top level"
msgstr "archive is not valid: multiple files found in zip file top level"

Expand Down Expand Up @@ -2533,10 +2533,14 @@ msgstr "dependency '%s' is not available"
msgid "destination already exists"
msgstr "destination already exists"

#: arduino/libraries/librariesmanager/install.go:74
#: arduino/libraries/librariesmanager/install.go:75
msgid "destination dir %s already exists, cannot install"
msgstr "destination dir %s already exists, cannot install"

#: arduino/libraries/librariesmanager/install.go:268
msgid "directory doesn't exist: %s"
msgstr "directory doesn't exist: %s"

#: arduino/discovery/discoverymanager/discoverymanager.go:112
msgid "discovery %[1]s process not started: %[2]w"
msgstr "discovery %[1]s process not started: %[2]w"
Expand Down Expand Up @@ -2599,7 +2603,7 @@ msgstr "error: %s and %s flags cannot be used together"
msgid "extracting archive: %s"
msgstr "extracting archive: %s"

#: arduino/libraries/librariesmanager/install.go:124
#: arduino/libraries/librariesmanager/install.go:125
msgid "extracting archive: %w"
msgstr "extracting archive: %w"

Expand Down Expand Up @@ -2694,7 +2698,7 @@ msgstr "getting tool dependencies for platform %[1]s: %[2]s"
msgid "importing sketch metadata: %s"
msgstr "importing sketch metadata: %s"

#: arduino/libraries/librariesmanager/install.go:91
#: arduino/libraries/librariesmanager/install.go:92
msgid "install directory not set"
msgstr "install directory not set"

Expand Down Expand Up @@ -2755,7 +2759,7 @@ msgstr "invalid empty library version: %s"
msgid "invalid empty option found"
msgstr "invalid empty option found"

#: arduino/libraries/librariesmanager/install.go:257
#: arduino/libraries/librariesmanager/install.go:258
msgid "invalid git url"
msgstr "invalid git url"

Expand Down Expand Up @@ -2819,22 +2823,18 @@ msgstr "key not found in settings"
msgid "keywords"
msgstr "keywords"

#: arduino/libraries/librariesmanager/install.go:162
#: arduino/libraries/librariesmanager/install.go:205
#: arduino/libraries/librariesmanager/install.go:163
#: arduino/libraries/librariesmanager/install.go:206
msgid "library %s already installed"
msgstr "library %s already installed"

#: arduino/libraries/librariesmanager/install.go:37
#: arduino/libraries/librariesmanager/install.go:38
msgid "library already installed"
msgstr "library already installed"

#: arduino/libraries/librariesmanager/install.go:276
msgid "library is not valid: missing file \"library.properties\""
msgstr "library is not valid: missing file \"library.properties\""

#: arduino/libraries/librariesmanager/install.go:271
msgid "library is not valid: missing header file \"%s\""
msgstr "library is not valid: missing header file \"%s\""
#: arduino/libraries/librariesmanager/install.go:305
msgid "library not valid"
msgstr "library not valid"

#: arduino/libraries/librariesmanager/librariesmanager.go:226
msgid "library path does not exist: %s"
Expand Down Expand Up @@ -2917,7 +2917,7 @@ msgstr "missing platform release %[1]s:%[2]s referenced by board %[3]s"
msgid "monitor release not found: %s"
msgstr "monitor release not found: %s"

#: arduino/libraries/librariesmanager/install.go:179
#: arduino/libraries/librariesmanager/install.go:180
#: arduino/resources/install.go:94
msgid "moving extracted archive to destination dir: %s"
msgstr "moving extracted archive to destination dir: %s"
Expand Down Expand Up @@ -3077,6 +3077,10 @@ msgstr "reading dir %[1]s: %[2]s"
msgid "reading directory %[1]s: %[2]s"
msgstr "reading directory %[1]s: %[2]s"

#: arduino/libraries/librariesmanager/install.go:278
msgid "reading directory %s content: %w"
msgstr "reading directory %s content: %w"

#: arduino/builder/sketch.go:76
msgid "reading file %[1]s: %[2]s"
msgstr "reading file %[1]s: %[2]s"
Expand All @@ -3093,11 +3097,11 @@ msgstr "reading inventory file: %w"
msgid "reading lib headers: %s"
msgstr "reading lib headers: %s"

#: arduino/libraries/libraries.go:234
#: arduino/libraries/libraries.go:235
msgid "reading lib src dir: %s"
msgstr "reading lib src dir: %s"

#: arduino/libraries/libraries.go:114
#: arduino/libraries/libraries.go:115
msgid "reading library headers: %w"
msgstr "reading library headers: %w"

Expand Down Expand Up @@ -3135,7 +3139,7 @@ msgstr "release not found"
msgid "removing corrupted archive file: %s"
msgstr "removing corrupted archive file: %s"

#: arduino/libraries/librariesmanager/install.go:94
#: arduino/libraries/librariesmanager/install.go:95
msgid "removing lib directory: %s"
msgstr "removing lib directory: %s"

Expand Down
14 changes: 7 additions & 7 deletions i18n/rice-box.go

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions test/test_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
# Test zip-path install
res = run_command(["lib", "install", "--zip-path", zip_path])
assert res.failed
assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
assert "library not valid" in res.stderr

lib_install_dir = Path(data_dir, "libraries", "lib-without-properties")
# Verifies library is not already installed
Expand All @@ -935,7 +935,7 @@ def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
# Test zip-path install
res = run_command(["lib", "install", "--zip-path", zip_path])
assert res.failed
assert 'library is not valid: missing file "library.properties"' in res.stderr
assert "library not valid" in res.stderr


def test_install_git_invalid_library(run_command, data_dir, downloads_dir):
Expand All @@ -962,7 +962,7 @@ def test_install_git_invalid_library(run_command, data_dir, downloads_dir):

res = run_command(["lib", "install", "--git-url", repo_dir], custom_env=env)
assert res.failed
assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
assert "library not valid" in res.stderr
assert not lib_install_dir.exists()

# Create another fake library repository
Expand All @@ -980,5 +980,5 @@ def test_install_git_invalid_library(run_command, data_dir, downloads_dir):

res = run_command(["lib", "install", "--git-url", repo_dir], custom_env=env)
assert res.failed
assert 'library is not valid: missing file "library.properties"' in res.stderr
assert "library not valid" in res.stderr
assert not lib_install_dir.exists()