Migrate packages #287
Migrate packages #287
Conversation
SummaryThis PR migrates utility packages from v1 to v2 module structure, consolidating certificateutil, profileutil, export, exportoptions, and plistutil packages into the main repository. The migration includes updating import paths across the codebase and replacing external dependencies like howett.net/plist with github.com/bitrise-io/go-plist. Walkthrough
|
| for _, certificate := range certificates { | ||
| if certificate != nil { | ||
| infos = append(infos, NewCertificateInfo(*certificate, nil)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🐛 Bug
Potential nil pointer dereference: The code checks if certificate != nil but then immediately dereferences it with *certificate. If the certificate pointer becomes nil between the check and dereference due to concurrent access, this could cause a panic.
🔄 Suggestion:
| for _, certificate := range certificates { | |
| if certificate != nil { | |
| infos = append(infos, NewCertificateInfo(*certificate, nil)) | |
| } | |
| } | |
| for _, certificate := range certificates { | |
| if certificate != nil { | |
| cert := *certificate | |
| infos = append(infos, NewCertificateInfo(cert, nil)) | |
| } | |
| } |
| for _, certificate := range certificates { | ||
| if certificate != nil { | ||
| infos = append(infos, NewCertificateInfo(*certificate, nil)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🐛 Bug
Potential nil pointer dereference: The code checks if certificate != nil but then immediately dereferences it with *certificate. If the certificate pointer becomes nil between the check and dereference due to concurrent access, this could cause a panic.
🔄 Suggestion:
| for _, certificate := range certificates { | |
| if certificate != nil { | |
| infos = append(infos, NewCertificateInfo(*certificate, nil)) | |
| } | |
| } | |
| for _, certificate := range certificates { | |
| if certificate != nil { | |
| cert := *certificate | |
| infos = append(infos, NewCertificateInfo(cert, nil)) | |
| } | |
| } |
| if len(certificates) != len(privateKeys) { | ||
| return nil, errors.New("pkcs12: different number of certificates and private keys found") | ||
| } |
There was a problem hiding this comment.
🐛 Bug
Inconsistent error handling: The function checks the length of certificates and privateKeys slices without first validating that both slices are non-nil, which could cause a panic if either slice is nil.
🔄 Suggestion:
| if len(certificates) != len(privateKeys) { | |
| return nil, errors.New("pkcs12: different number of certificates and private keys found") | |
| } | |
| if certificates == nil || privateKeys == nil { | |
| return nil, errors.New("pkcs12: certificates or private keys slice is nil") | |
| } | |
| if len(certificates) != len(privateKeys) { | |
| return nil, errors.New("pkcs12: different number of certificates and private keys found") | |
| } |
| pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)` | ||
| matches := regexp.MustCompile(pattern).FindAllString(out, -1) |
There was a problem hiding this comment.
🐛 Bug
Regex compilation inefficiency: The regex pattern is compiled inside the function on every call. For better performance and to avoid potential regex compilation errors at runtime, this should be compiled once as a package-level variable.
🔄 Suggestion:
| pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)` | |
| matches := regexp.MustCompile(pattern).FindAllString(out, -1) | |
| var certificatePattern = regexp.MustCompile(`(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`) | |
| func normalizeFindCertificateOut(out string) ([]string, error) { | |
| certificateContents := []string{} | |
| matches := certificatePattern.FindAllString(out, -1) |
| missingEntitlements := []string{} | ||
|
|
||
| for key := range targetEntitlements { | ||
| _, known := profileutil.KnownProfileCapabilitiesMap[profileType][key] |
There was a problem hiding this comment.
🐛 Bug
Potential nil pointer dereference: The code accesses profileutil.KnownProfileCapabilitiesMap[profileType][key] without checking if profileutil.KnownProfileCapabilitiesMap[profileType] exists first. This will cause a panic if an unsupported profileType (like ProfileTypeTvOs) is passed.
🔄 Suggestion:
| _, known := profileutil.KnownProfileCapabilitiesMap[profileType][key] | |
| profileCapabilities, exists := profileutil.KnownProfileCapabilitiesMap[profileType] | |
| if !exists { | |
| continue | |
| } | |
| _, known := profileCapabilities[key] |
| } | ||
|
|
||
| func installedCodesigningCertificateNamesFromOutput(out string) ([]string, error) { | ||
| pettern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"` |
There was a problem hiding this comment.
🐛 Bug
Variable name typo: 'pettern' should be 'pattern'. This will cause a compilation error as the variable is used on the next line but never declared correctly.
🔄 Suggestion:
| pettern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"` | |
| pattern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"` |
| missingEntitlements := []string{} | ||
|
|
||
| for key := range targetEntitlements { | ||
| _, known := profileutil.KnownProfileCapabilitiesMap[profileType][key] |
There was a problem hiding this comment.
🐛 Bug
Potential map access panic when profileType doesn't exist as a key in KnownProfileCapabilitiesMap. If an unsupported profile type (like ProfileTypeTvOs) is passed, this will cause a runtime panic when trying to access the nested map.
🔄 Suggestion:
| _, known := profileutil.KnownProfileCapabilitiesMap[profileType][key] | |
| capabilitiesMap, exists := profileutil.KnownProfileCapabilitiesMap[profileType] | |
| if !exists { | |
| continue | |
| } | |
| _, known := capabilitiesMap[key] |
| return nil, err | ||
| } | ||
|
|
||
| pattern := filepath.Join(pathutil.EscapeGlobPath(absProvProfileDirPath), "*"+ext) |
There was a problem hiding this comment.
🐛 Bug
Function pathutil.EscapeGlobPath does not exist in the github.com/bitrise-io/go-utils/pathutil package. This will cause a compilation error when the code is built.
🔄 Suggestion:
| pattern := filepath.Join(pathutil.EscapeGlobPath(absProvProfileDirPath), "*"+ext) | |
| pattern := filepath.Join(absProvProfileDirPath, "*"+ext) |
No description provided.