Merge pull request #3271 from cristiand391/disable-preview-long-url

Disable preview option in prompts if URL size is too long
This commit is contained in:
Mislav Marohnić 2021-03-30 19:34:28 +02:00 committed by GitHub
commit 79219c7741
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 81 additions and 43 deletions

View file

@ -157,22 +157,20 @@ func createRun(opts *CreateOptions) (err error) {
tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false)
var openURL string
if opts.WebMode {
openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
if opts.Title != "" || opts.Body != "" || tb.HasMetadata() {
if len(opts.Projects) > 0 {
var err error
tb.Projects, err = api.ProjectNamesToPaths(apiClient, baseRepo, tb.Projects)
if err != nil {
return fmt.Errorf("could not add to project: %w", err)
}
}
openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
openURL, err = generatePreviewURL(apiClient, baseRepo, tb)
if err != nil {
return
}
if !utils.ValidURL(openURL) {
err = fmt.Errorf("cannot open in browser: maximum URL length exceeded")
return
}
} else if ok, _ := tpl.HasTemplates(); ok {
openURL += "/choose"
openURL = ghrepo.GenerateRepoURL(baseRepo, "issues/new/choose")
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL))
@ -240,7 +238,13 @@ func createRun(opts *CreateOptions) (err error) {
}
}
action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage())
openURL, err = generatePreviewURL(apiClient, baseRepo, tb)
if err != nil {
return
}
allowPreview := !tb.HasMetadata() && utils.ValidURL(openURL)
action, err = prShared.ConfirmSubmission(allowPreview, repo.ViewerCanTriage())
if err != nil {
err = fmt.Errorf("unable to confirm: %w", err)
return
@ -277,18 +281,6 @@ func createRun(opts *CreateOptions) (err error) {
}
if action == prShared.PreviewAction {
if len(tb.Projects) > 0 {
var err error
tb.Projects, err = api.ProjectNamesToPaths(apiClient, repo, tb.Projects)
if err != nil {
return fmt.Errorf("could not add to project: %w", err)
}
}
openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
if err != nil {
return
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL))
}
@ -320,3 +312,8 @@ func createRun(opts *CreateOptions) (err error) {
return
}
func generatePreviewURL(apiClient *api.Client, baseRepo ghrepo.Interface, tb shared.IssueMetadataState) (string, error) {
openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
return prShared.WithPrAndIssueQueryParams(apiClient, baseRepo, openURL, tb)
}

View file

@ -532,6 +532,15 @@ func TestIssueCreate_web(t *testing.T) {
assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", output.BrowsedURL)
}
func TestIssueCreate_webLongURL(t *testing.T) {
longBodyFile := filepath.Join(t.TempDir(), "long-body.txt")
err := ioutil.WriteFile(longBodyFile, make([]byte, 9216), 0600)
require.NoError(t, err)
_, err = runCommand(nil, true, fmt.Sprintf("-F '%s' --web", longBodyFile))
require.EqualError(t, err, "cannot open in browser: maximum URL length exceeded")
}
func TestIssueCreate_webTitleBody(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)

View file

@ -190,6 +190,8 @@ func createRun(opts *CreateOptions) (err error) {
return
}
var openURL string
if opts.WebMode {
if !opts.Autofill {
state.Title = opts.Title
@ -199,7 +201,15 @@ func createRun(opts *CreateOptions) (err error) {
if err != nil {
return
}
return previewPR(*opts, *ctx, *state)
openURL, err = generateCompareURL(*ctx, *state)
if err != nil {
return
}
if !utils.ValidURL(openURL) {
err = fmt.Errorf("cannot open in browser: maximum URL length exceeded")
return
}
return previewPR(*opts, openURL)
}
if opts.TitleProvided {
@ -291,8 +301,14 @@ func createRun(opts *CreateOptions) (err error) {
}
}
openURL, err = generateCompareURL(*ctx, *state)
if err != nil {
return
}
allowPreview := !state.HasMetadata() && utils.ValidURL(openURL)
allowMetadata := ctx.BaseRepo.ViewerCanTriage()
action, err := shared.ConfirmSubmission(!state.HasMetadata(), allowMetadata)
action, err := shared.ConfirmSubmission(allowPreview, allowMetadata)
if err != nil {
return fmt.Errorf("unable to confirm: %w", err)
}
@ -327,7 +343,7 @@ func createRun(opts *CreateOptions) (err error) {
}
if action == shared.PreviewAction {
return previewPR(*opts, *ctx, *state)
return previewPR(*opts, openURL)
}
if action == shared.SubmitAction {
@ -639,20 +655,7 @@ func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataS
return nil
}
func previewPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataState) error {
if len(state.Projects) > 0 {
var err error
state.Projects, err = api.ProjectNamesToPaths(ctx.Client, ctx.BaseRepo, state.Projects)
if err != nil {
return fmt.Errorf("could not add to project: %w", err)
}
}
openURL, err := generateCompareURL(ctx, state)
if err != nil {
return err
}
func previewPR(opts CreateOptions, openURL string) error {
if opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL))
}
@ -750,7 +753,7 @@ func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState) (str
ctx.BaseRepo,
"compare/%s...%s?expand=1",
url.QueryEscape(ctx.BaseBranch), url.QueryEscape(ctx.HeadBranchLabel))
url, err := shared.WithPrAndIssueQueryParams(u, state)
url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.BaseRepo, u, state)
if err != nil {
return "", err
}

View file

@ -835,6 +835,26 @@ func TestPRCreate_web(t *testing.T) {
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL)
}
func TestPRCreate_webLongURL(t *testing.T) {
longBodyFile := filepath.Join(t.TempDir(), "long-body.txt")
err := ioutil.WriteFile(longBodyFile, make([]byte, 9216), 0600)
require.NoError(t, err)
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoInfoResponse("OWNER", "REPO", "master")
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git status --porcelain`, 0, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
_, err = runCommand(http, nil, "feature", false, fmt.Sprintf("--body-file '%s' --web --head=feature", longBodyFile))
require.EqualError(t, err, "cannot open in browser: maximum URL length exceeded")
}
func TestPRCreate_webProject(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)

View file

@ -10,7 +10,7 @@ import (
"github.com/cli/cli/pkg/githubsearch"
)
func WithPrAndIssueQueryParams(baseURL string, state IssueMetadataState) (string, error) {
func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState) (string, error) {
u, err := url.Parse(baseURL)
if err != nil {
return "", err
@ -29,7 +29,11 @@ func WithPrAndIssueQueryParams(baseURL string, state IssueMetadataState) (string
q.Set("labels", strings.Join(state.Labels, ","))
}
if len(state.Projects) > 0 {
q.Set("projects", strings.Join(state.Projects, ","))
projectPaths, err := api.ProjectNamesToPaths(client, baseRepo, state.Projects)
if err != nil {
return "", fmt.Errorf("could not add to project: %w", err)
}
q.Set("projects", strings.Join(projectPaths, ","))
}
if len(state.Milestones) > 0 {
q.Set("milestone", state.Milestones[0])

View file

@ -78,3 +78,8 @@ func DisplayURL(urlStr string) string {
}
return u.Hostname() + u.Path
}
// Maximum length of a URL: 8192 bytes
func ValidURL(urlStr string) bool {
return len(urlStr) < 8192
}