diff --git a/e2e/url.bats b/e2e/url.bats index 250253e94..488933d59 100644 --- a/e2e/url.bats +++ b/e2e/url.bats @@ -133,6 +133,19 @@ load test_helper assert_output_contains "ID required" } +@test "basecamp url parse names a document in a folder, not the folder" { + run basecamp url parse "https://3.basecamp.com/123/buckets/456/vaults/111/documents/789" --json + assert_success + assert_json_value ".data.type" "documents" + assert_json_value ".data.recording_id" "789" +} + +@test "basecamp url parse refuses a path to another item under the one it names" { + run basecamp url parse "https://3.basecamp.com/123/buckets/456/messages/789/boosts/5" + assert_failure + assert_output_contains "Could not parse URL" +} + @test "basecamp url parse fails for non-Basecamp URL" { run basecamp url parse "https://github.com/test/repo" assert_failure diff --git a/internal/urlarg/urlarg.go b/internal/urlarg/urlarg.go index 366cad4ca..4380eb8d2 100644 --- a/internal/urlarg/urlarg.go +++ b/internal/urlarg/urlarg.go @@ -22,6 +22,35 @@ var pingAtRe = regexp.MustCompile(`/circles/(\d+)@(\d+)`) // Basecamp links to for the same Ping, and drops the my/sidebar prefix. var pingSidebarRe = regexp.MustCompile(`/my/sidebar(/circles/\d+)`) +// slashRunRe matches a run of slashes, which Rails routes as one. +var slashRunRe = regexp.MustCompile(`/{2,}`) + +// jsonSuffixRe matches the .json an API URL's path ends in. +var jsonSuffixRe = regexp.MustCompile(`^([^?#]*)\.json([?#]|$)`) + +// itemPaths are paths whose segments past an item name another item, each +// rewritten to that item's own path as ${1}${2}: a child nested under its +// parent, which bc3 also serves at top level, and an upload's download and +// preview links. bc3 links those to a version even when it is the latest, so +// they name the upload; a download's filename, which may be all digits, goes +// with them. +var itemPaths = []*regexp.Regexp{ + regexp.MustCompile(`/vaults/\d+(/(?:vaults|documents|google_documents|cloud_files|uploads)/\d+)([/?#]|$)`), + regexp.MustCompile(`/todolists/\d+(/todos/\d+)([/?#]|$)`), + regexp.MustCompile(`/questionnaires/\d+(/questions/\d+)([/?#]|$)`), + regexp.MustCompile(`(/uploads/\d+)(?:/versions/\d+)?/download/[^?#]+([?#]|$)`), + regexp.MustCompile(`(/uploads/\d+)/versions/\d+/representations/[^/?#]+([?#]|$)`), +} + +// structuralPath is the whole path of each shape the router's structural +// fallback recognizes. The fallback matches only a prefix of the path. +var structuralPath = regexp.MustCompile(`^/\d+(?:/projects/\d+|/buckets/\d+/(?:card_tables/[^/]+/\d+|[^/]+(?:/\d+)?))?$`) + +// trailingWords matches the segments after a path's last ID when each starts +// with a letter: a page of that item, such as edit or dates/edit. A segment +// that starts otherwise, such as 222.json or %32%32, may hold an ID. +var trailingWords = regexp.MustCompile(`(/\d+)(?:/[A-Za-z][^/?#]*)+([?#]|$)`) + // Parsed represents components extracted from a Basecamp URL. type Parsed struct { AccountID string @@ -45,20 +74,80 @@ func IsURL(input string) bool { return router.Match(normalized) != nil } -// normalize rewrites the web @-forms into routable URLs. Chat lines become the +// normalize rewrites the web @-forms and itemPaths into routable URLs, and +// drops a .json suffix and the slashes Rails ignores. Chat lines become the // slash form; a Ping line loses its @line, which the second result returns. func normalize(input string) (normalized, pingLineID string) { - normalized = chatAtRe.ReplaceAllString(input, "${1}/lines/${2}") + normalized = chatAtRe.ReplaceAllString(squeezeSlashes(input), "${1}/lines/${2}") normalized = pingSidebarRe.ReplaceAllString(normalized, "${1}") if m := pingAtRe.FindStringSubmatch(normalized); m != nil { pingLineID = m[2] normalized = pingAtRe.ReplaceAllString(normalized, "/circles/${1}") } + normalized = jsonSuffixRe.ReplaceAllString(normalized, "${1}${2}") + for _, re := range itemPaths { + normalized = re.ReplaceAllString(normalized, "${1}${2}") + } return normalized, pingLineID } +// squeezeSlashes collapses each run of slashes in a URL's path to one and drops +// a trailing slash, as Rails does when routing, so that a suffix or ID before +// them still ends the path. +func squeezeSlashes(rawURL string) string { + end := strings.IndexAny(rawURL, "?#") + if end == -1 { + end = len(rawURL) + } + start := 0 + if i := strings.Index(rawURL[:end], "://"); i != -1 { + start = i + len("://") + } else if strings.HasPrefix(rawURL, "//") { + start = len("//") + } + path := strings.TrimRight(slashRunRe.ReplaceAllString(rawURL[start:end], "/"), "/") + return rawURL[:start] + path + rawURL[end:] +} + +// urlPath returns the path the router matches: the URL without its scheme, +// host, query, or fragment. +func urlPath(rawURL string) string { + path, _, _ := strings.Cut(rawURL, "#") + path, _, _ = strings.Cut(path, "?") + if _, afterScheme, ok := strings.Cut(path, "://"); ok { + path = afterScheme + } else { + path = strings.TrimPrefix(path, "//") + } + if !strings.HasPrefix(path, "/") { + slash := strings.Index(path, "/") + if slash == -1 { + return "" + } + path = path[slash:] + } + return path +} + +// match routes a normalized URL, refusing a structural match that leaves an ID +// unconsumed. Words after the item's ID name the item: the URL is routed again +// without them, so a page of an API item, such as a chat line's edit page, +// still names that item. +func match(normalized string) *basecamp.Match { + for _, u := range []string{normalized, trailingWords.ReplaceAllString(normalized, "${1}${2}")} { + m := router.Match(u) + if m == nil || m.Source == basecamp.MatchedAPI || structuralPath.MatchString(urlPath(u)) { + return m + } + } + return nil +} + // Parse extracts IDs from a Basecamp URL. -// Returns nil if the input is not a valid Basecamp URL. +// Returns nil if the input is not a valid Basecamp URL, if its path goes past +// the item it names to another one Parse does not model, or if the ID it ends +// in is a person's or an embed's, so that callers refuse it rather than act on +// the wrong recording. // // Supported URL patterns: // - https://3.basecamp.com/{account}/buckets/{bucket}/{type}/{id} @@ -69,16 +158,26 @@ func normalize(input string) (normalized, pingLineID string) { // - https://3.basecamp.com/{account}/circles/{ping}@{lineId} // - https://3.basecamp.com/{account}/my/sidebar/circles/{ping}, with or without @{lineId} // - https://3.basecamp.com/{account}/buckets/{bucket}/schedule_entries/{id}/occurrences/{date} +// - https://3.basecamp.com/{account}/buckets/{bucket}/vaults/{folder}/documents/{id}, and the +// other children bc3 nests under folders, to-do lists, and questionnaires +// - any of these followed by segments with no ID, such as /edit, and an upload +// followed by /download/{filename} or a version's /download/{filename} or +// /representations/{key} // - https://3.basecamp.com/{account}/projects/{project} func Parse(input string) *Parsed { // Normalize the Basecamp web @-forms (chats/789@111, circles/456@111) // into routable URLs before routing. normalized, pingLineID := normalize(input) - m := router.Match(normalized) + m := match(normalized) if m == nil { return nil } + // questions/{id}/answers/by/{person}, people/{person}/out_of_office and the + // reports by person end in a person's ID, and embeds/{sgid} in an sgid. + if m.Params["sgid"] != "" || (m.Params["personId"] != "" && m.PathType != "people") { + return nil + } if pingLineID != "" { pingID := m.Params["pingId"] diff --git a/internal/urlarg/urlarg_trailing_test.go b/internal/urlarg/urlarg_trailing_test.go new file mode 100644 index 000000000..a2d9eb69c --- /dev/null +++ b/internal/urlarg/urlarg_trailing_test.go @@ -0,0 +1,243 @@ +package urlarg + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Segments past an item's ID name that item unless one of them is another ID. +// Then the URL names that other item only through a path Parse knows, and is +// otherwise refused, never resolved to the parent. +func TestParseTrailingSegments(t *testing.T) { + const base = "https://app.basecamp.com/99999/buckets/123" + tests := []struct { + name string + input string + want *Parsed + }{ + { + name: "occurrence edit page keeps the occurrence date", + input: base + "/schedule_entries/456/occurrences/20261022/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "schedule_entries", RecordingID: "456", OccurrenceDate: "20261022"}, + }, + { + name: "document in a folder names the document", + input: base + "/vaults/111/documents/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "456"}, + }, + { + name: "document edit page in a folder names the document", + input: base + "/vaults/111/documents/456/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "456"}, + }, + { + name: "upload in a folder names the upload", + input: base + "/vaults/111/uploads/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "Google document in a folder names the Google document", + input: base + "/vaults/111/google_documents/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "google_documents", RecordingID: "456"}, + }, + { + name: "cloud file in a folder names the cloud file", + input: base + "/vaults/111/cloud_files/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "cloud_files", RecordingID: "456"}, + }, + { + name: "folder in a folder names the inner folder", + input: base + "/vaults/111/vaults/222", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "vaults", RecordingID: "222"}, + }, + { + name: "to-do in a to-do list names the to-do", + input: base + "/todolists/111/todos/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "456"}, + }, + { + name: "question in a questionnaire names the question", + input: base + "/questionnaires/111/questions/456", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "questions", RecordingID: "456"}, + }, + { + name: "chat line edit page names the line, not the chat", + input: base + "/chats/789/lines/111/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "lines", RecordingID: "111", CampfireID: "789"}, + }, + { + name: "to-do edit page names the to-do", + input: base + "/todos/789/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789"}, + }, + { + name: "edit page with a trailing slash, query and comment fragment", + input: base + "/todos/789/edit/?x=1#__recording_5", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789", CommentID: "5"}, + }, + { + name: "card edit page names the card", + input: base + "/card_tables/cards/789/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "cards", RecordingID: "789"}, + }, + { + name: "upload download link names the upload", + input: base + "/uploads/456/download/Q3%20report.pdf", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "upload version download link names the upload", + input: base + "/uploads/456/versions/7/download/a.pdf", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "upload version download link with a query names the upload", + input: base + "/uploads/456/versions/7/download/Q3%20report.pdf?disposition=attachment", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "upload version preview names the upload", + input: base + "/uploads/456/versions/7/representations/eyJfcmFpbHMiOnt9fQ==--a1b2", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "new-version page names the upload", + input: base + "/uploads/456/versions/new", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "person edit page names the person", + input: "https://app.basecamp.com/99999/people/5/edit", + want: &Parsed{AccountID: "99999", Type: "people", RecordingID: "5"}, + }, + { + name: "to-do dates edit page names the to-do", + input: base + "/todos/789/dates/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789"}, + }, + { + name: "comment editor page names the comment", + input: base + "/comments/789/compositions/edit", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "comments", RecordingID: "789"}, + }, + { + name: "unknown page under a to-do names the to-do", + input: base + "/todos/789/unknown", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789"}, + }, + { + name: "occurrence status path keeps the occurrence date", + input: base + "/schedule_entries/456/occurrences/20261022/status/trashed", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "schedule_entries", RecordingID: "456", OccurrenceDate: "20261022"}, + }, + { + name: "new-document page in a folder names the folder", + input: base + "/vaults/111/documents/new", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "vaults", RecordingID: "111"}, + }, + { + name: "unknown page under a project names the project", + input: "https://app.basecamp.com/99999/projects/123/unknown", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "project"}, + }, + { + name: "JSON document in a folder names the document", + input: base + "/vaults/111/documents/222.json", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "222"}, + }, + { + name: "JSON document in a folder with a comment fragment names the comment", + input: base + "/vaults/111/documents/222.json#__recording_789", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "222", CommentID: "789"}, + }, + { + name: "JSON document in a folder with a trailing slash, query and comment fragment names the comment", + input: base + "/vaults/111/documents/222.json/?x=1#__recording_789", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "222", CommentID: "789"}, + }, + { + name: "JSON card column with a trailing slash, query and comment fragment names the column", + input: base + "/card_tables/lists/222.json/?x=1#__recording_789", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "columns", RecordingID: "222", CommentID: "789"}, + }, + { + name: "document in a folder after a doubled slash names the document", + input: base + "/vaults/111//documents/222?x=1#__recording_789", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "documents", RecordingID: "222", CommentID: "789"}, + }, + { + name: "to-do edit page after a doubled slash names the to-do", + input: base + "/todos/789//edit?x=1#__recording_5", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789", CommentID: "5"}, + }, + { + name: "to-do ID after a doubled slash names the to-do", + input: base + "/todos//789", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "todos", RecordingID: "789"}, + }, + { + name: "download link with an all-digit filename and a trailing slash names the upload", + input: base + "/uploads/456/download/12345/?x=1#y", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "version download link with a digit-leading filename and a trailing slash names the upload", + input: base + "/uploads/456/versions/7/download/222.json/?disposition=attachment", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "download link with digits in the filename names the upload", + input: base + "/uploads/456/download/report2024.pdf", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "download link with an all-digit filename names the upload", + input: base + "/uploads/456/download/12345", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "download link with an all-digit filename and extension names the upload", + input: base + "/uploads/456/download/12345.pdf", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + { + name: "version download link with an all-digit filename names the upload", + input: base + "/uploads/456/versions/7/download/12345", + want: &Parsed{AccountID: "99999", ProjectID: "123", Type: "uploads", RecordingID: "456"}, + }, + {name: "upload version as JSON", input: base + "/uploads/456/versions/7.json"}, + {name: "upload version as JSON with a trailing slash, query and fragment", input: base + "/uploads/456/versions/7.json/?x=1#__recording_5"}, + {name: "document in a folder with another suffix", input: base + "/vaults/111/documents/222.html?x=1#y"}, + {name: "document in a folder as uppercase JSON", input: base + "/vaults/111/documents/222.JSON"}, + {name: "document in a folder with a percent-encoded ID", input: base + "/vaults/111/documents/%32%32%32"}, + {name: "document in a folder with a signed ID", input: base + "/vaults/111/documents/+222"}, + {name: "answers by a person end in the person's ID", input: base + "/questions/111/answers/by/789"}, + {name: "a person's out-of-office ends in the person's ID", input: "https://app.basecamp.com/99999/people/5/out_of_office"}, + {name: "to-dos assigned to a person end in the person's ID", input: "https://app.basecamp.com/99999/reports/todos/assigned/5"}, + {name: "a person's progress ends in the person's ID", input: "https://app.basecamp.com/99999/reports/users/progress/5"}, + {name: "an embed ends in its sgid", input: "https://app.basecamp.com/99999/embeds/BAh7abc"}, + {name: "folder child the parser does not model", input: base + "/vaults/111/children/222"}, + {name: "answer under a question", input: base + "/questions/111/answers/2"}, + {name: "boost under a message", input: base + "/messages/789/boosts/5"}, + {name: "step under a card", input: base + "/card_tables/cards/789/steps/5"}, + {name: "upload version page bc3 does not serve", input: base + "/uploads/456/versions/7"}, + {name: "bucket without a resource", input: base}, + {name: "unknown account-level page", input: "https://app.basecamp.com/99999/whatever/5/deep"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, Parse(tt.input)) + }) + } +} + +// A refused URL is still a URL, so callers report it as one rather than +// treating it as a name or an ID. +func TestRefusedURLIsStillURL(t *testing.T) { + url := "https://app.basecamp.com/99999/buckets/123/vaults/111/children/222" + assert.True(t, IsURL(url)) + assert.Nil(t, Parse(url)) + assert.Equal(t, url, ExtractID(url)) +}