notify-slack: Migrate to new upload APIs

[joverlee521]

Description of proposed changes

The files.upload API will be sunset on March 11, 2025.¹ This commit follows Slack's "Uploading files" docs² to migrate to the new upload APIs, files.getUploadURLExternal and files.completeUploadExternal.

The usage of the script does not change. However, the new API requires the channels to be provided as their channel IDs instead of channel names, so we will have to update the SLACK_CHANNELS envvar for various pathogen workflows. Channel ids are more stable anyways since channels can be renamed.

¹ https://api.slack.com/changelog/2024-04-a-better-way-to-upload-files-is-here-to-stay
² https://api.slack.com/messaging/files#uploading_files

Related issue(s)

Resolves #46

Checklist

• Checks pass
• If adding a script, add an entry for it in the README.

[tsibley]

Why the printf?

[joverlee521]

Ah, I added to force it to be an int because the Slack API kept returning the error

[ERROR] must provide a number [json-pointer:\/length]

---

Looks like I can remove it if I stick with --form, but need it when using --form-string as you suggested.

[tsibley]

Hmm. Bash doesn't have the concept of int vs. string types for values. Something else must have been happening.

[tsibley]

I dug into this to verify/amend/correct my mental model of how things work.

The short of it is that we should probably just leave the printf, though adding brief comment saying why it's there might be nice.

Ok, now for the long of it.

Consider this program

set -euo pipefail -x

upload_file="$(mktemp -t upload-file-XXXXXX)"
trap "rm -f '$upload_file'" EXIT

seq 1 100 > "$upload_file"

printf=$(printf '%d' "$(<"$upload_file" wc -c)")

curl https://httpbin.org/post \
    --form-string printf-form-string="$printf" \
    --form printf-form="$printf"

noprintf=$(<"$upload_file" wc -c)

curl https://httpbin.org/post \
    --form-string noprintf-form-string="$noprintf" \
    --form noprintf-form="$noprintf"

run on Linux

++ mktemp -t upload-file-XXXXXX
+ upload_file=/tmp/upload-file-JJx2PL
+ trap 'rm -f '\''/tmp/upload-file-JJx2PL'\''' EXIT
+ seq 1 100
+++ wc -c
++ printf %d 292
+ printf=292
+ curl https://httpbin.org/post --form-string printf-form-string=292 --form printf-form=292
{
  "args": {},
  "data": "",
  "files": {},
  "form": {
    "printf-form": "292",
    "printf-form-string": "292"
  },
  "headers": {
    "Accept": "*/*",
    "Content-Length": "277",
    "Content-Type": "multipart/form-data; boundary=------------------------f6zvS2CymeD4Oo9HrOqKM7",
    "Host": "httpbin.org",
    "User-Agent": "curl/8.5.0",
    "X-Amzn-Trace-Id": "Root=1-67c13f0b-2b4def584aeb84e87a66bce2"
  },
  "json": null,
  "origin": "67.183.183.108",
  "url": "https://httpbin.org/post"
}
++ wc -c
+ noprintf=292
+ curl https://httpbin.org/post --form-string noprintf-form-string=292 --form noprintf-form=292
{
  "args": {},
  "data": "",
  "files": {},
  "form": {
    "noprintf-form": "292",
    "noprintf-form-string": "292"
  },
  "headers": {
    "Accept": "*/*",
    "Content-Length": "281",
    "Content-Type": "multipart/form-data; boundary=------------------------6waQojCdHJKO6enjWylQNZ",
    "Host": "httpbin.org",
    "User-Agent": "curl/8.5.0",
    "X-Amzn-Trace-Id": "Root=1-67c13f0b-1790ea454ca586093be371de"
  },
  "json": null,
  "origin": "67.183.183.108",
  "url": "https://httpbin.org/post"
}
+ rm -f /tmp/upload-file-JJx2PL

vs. macOS

++ mktemp -t upload-file-XXXXXX
+ upload_file=/var/folders/tt/crsvc10961q94pdcvrgjmmt40000gn/T/upload-file-XXXXXX.l9WDtwp4h0
+ trap 'rm -f '\''/var/folders/tt/crsvc10961q94pdcvrgjmmt40000gn/T/upload-file-XXXXXX.l9WDtwp4h0'\''' EXIT
+ seq 1 100
+++ wc -c
++ printf %d '     292'
+ printf=292
+ curl https://httpbin.org/post --form-string printf-form-string=292 --form printf-form=292
{
  "args": {},
  "data": "",
  "files": {},
  "form": {
    "printf-form": "292",
    "printf-form-string": "292"
  },
  "headers": {
    "Accept": "*/*",
    "Content-Length": "277",
    "Content-Type": "multipart/form-data; boundary=------------------------63IPEWq92HeiyJYCzjeqcw",
    "Host": "httpbin.org",
    "User-Agent": "curl/8.4.0",
    "X-Amzn-Trace-Id": "Root=1-67c13f9d-3993b0bc6a71e4425d0bcd4c"
  },
  "json": null,
  "origin": "67.183.183.108",
  "url": "https://httpbin.org/post"
}
++ wc -c
+ noprintf='     292'
+ curl https://httpbin.org/post --form-string 'noprintf-form-string=     292' --form 'noprintf-form=     292'
{
  "args": {},
  "data": "",
  "files": {},
  "form": {
    "noprintf-form": "292",
    "noprintf-form-string": "     292"
  },
  "headers": {
    "Accept": "*/*",
    "Content-Length": "286",
    "Content-Type": "multipart/form-data; boundary=------------------------nodcbRgiLh6Q9vx9WyE1GS",
    "Host": "httpbin.org",
    "User-Agent": "curl/8.4.0",
    "X-Amzn-Trace-Id": "Root=1-67c13f9d-790482e24dbd7ef005258256"
  },
  "json": null,
  "origin": "67.183.183.108",
  "url": "https://httpbin.org/post"
}
+ rm -f /var/folders/tt/crsvc10961q94pdcvrgjmmt40000gn/T/upload-file-XXXXXX.l9WDtwp4h0

This is a macOS/BSD wc vs. GNU coreutils wc thing, combined with --form-string submitting exactly what's given (generally what you want) vs. --form interpreting and altering what's given (generally muddies the water). The macOS wc is emitting leading spaces (for filename vs. count separation, but there's no filename for stdin) and --form trims leading spaces. The GNU coreutils wc only emits the count for stdin, without whitespace.

I'd almost made a review comment originally about not using wc -c to get file size and using stat instead, because I thought a) it'd be slow (e.g. to read the whole file) and b) have portability problems around options or output format.

When I looked into (a) though (with strace), I found that GNU coreutils wc issues an fstat() system call to try to get the size if it can, which works when stdin is redirected from a file.

$ seq 1 100 > /tmp/d
$ strace wc -c < /tmp/d
execve("/bin/wc", ["wc", "-c"], 0x7ffdca82e698 /* 103 vars */) = 0
[…]
fstat(0, {st_mode=S_IFREG|0664, st_size=292, ...}) = 0
lseek(0, 0, SEEK_CUR)                   = 0
lseek(0, 292, SEEK_CUR)                 = 292
[…]
write(1, "292\n", 4292)                 = 4
[…]

It only falls back to reading the whole input when the fstat() fails to report a size, which it will on a pipe.

$ cat /tmp/d | strace wc -c
execve("/bin/wc", ["wc", "-c"], 0x7ffe6a8201e8 /* 103 vars */) = 0
[…]
fstat(0, {st_mode=S_IFIFO|0600, st_size=0, ...}) = 0
fadvise64(0, 0, 0, POSIX_FADV_SEQUENTIAL) = -1 ESPIPE (Illegal seek)
read(0, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 16384) = 292
read(0, "", 16384)                      = 0
[…]
write(1, "292\n", 4292)                 = 4
[…]

This is more clever than I thought it might be, and that's good! (Not entirely surprising, as GNU cat is also much more complex (and optimized) than meets the eye.)

When I looked into (b), I saw that -c seemed semantically identical (bytes, not chars) across BSD/GNU, and I figured the output format wasn't an issue since you'd tested it. (I didn't grok that the printf was trimming the whitespace, as I didn't know it'd do that!)

Still, I thought maybe stat would be better, so I was going to weakly suggest stat --format=%s "$upload_file" to get the size. When I checked on macOS though, the interface of stat is very different: the equivalent command would be stat -f %z "$upload_file", and there was no way to write a command that worked the same on both. At that point I threw up my hands and figured the wc -c you had was better than using Perl as a Unix-compatibility shim (e.g. perl -e 'print -s $ARGV[0]' "$upload_file"). So I deleted my review comment before submitting.

[joverlee521]

Wow, thanks for digging into this and giving a detailed explanation! I added a comment in the notify-slack in cd6d31a.

[joverlee521]

Tested locally by sending to the #scratch channel.