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

1) Fixed URL segment separator issue. in GetPackageByName() preventing use on Microsoft Windows. #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wvdvegt
Copy link

@wvdvegt wvdvegt commented Sep 28, 2020

Should fix the issue.

1) Fixed URL segment separator issue. in GetPackageByName() preventing use on Microsoft Windows.
@@ -81,7 +81,7 @@ func NewPackagist(instance string, httpClient *http.Client) (*PackagistClient, e

// GetPackageByName returns a package by a given name
func (c *PackagistClient) GetPackageByName(name string) (*PackagistPackage, *http.Response, error) {
u := fmt.Sprintf("%s/packages%s.json", c.url.String(), filepath.Clean("/"+name))
u := fmt.Sprintf("%s/packages%s.json", c.url.String(), strings.Replace(filepath.Clean("/"+name), "\\", "/", -1))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #6 (comment) you pointed out that the issue might be the usage of filepath.Clean. What do you think about removing the filepath.Clean reference and replacing it with an URL fitting pattern? e.g. typical URL encoding?
Have a look at https://www.urlencoder.io/golang/ to check if one of those might be a good choice.

@wvdvegt
Copy link
Author

wvdvegt commented Sep 29, 2020

Hi
My experience with go is only 1 hr, so code might be improved. The main advantage of filetpath.Clean is it removes some unwanted stuff from the package name. But there might be better ways.

@wvdvegt
Copy link
Author

wvdvegt commented Oct 1, 2020

Hi

The following replacement appears to work on Windows (and using net/url):

	//u := fmt.Sprintf("%s/packages%s.json", c.url.String(), strings.Replace(filepath.Clean("/"+name), "\\", "/", -1))
	u, err := url.Parse(c.url.String())

	// Note: not sure this code is necceasry (as the url.Parse can't fail):
	//if err != nil {
	//	return nil, nil, err
	//}

	u.Path += "/packages"
	u.Path += fmt.Sprintf("/%s.json", name)

	resp, err := c.httpClient.Get(u.String())

I'm not sure the first if statement is necessary as the packagist.org URL is probably hardcoded and not modifiable using a command line switch (so the url.Parse() can't fail.

Personally I think this is indeed a better solution (properly building the url instead of stitching it together as a string)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants