common-fate / granted

The easiest way to access your cloud.
https://granted.dev
MIT License
957 stars 90 forks source link

Fix backwards invocation of io.Copy #589

Closed sosheskaz closed 6 months ago

sosheskaz commented 6 months ago

What changed?

Reverse args to io.Copy

Why?

io.Copy has the opposite order you would expect: https://pkg.go.dev/io#Copy

This causes issues for Linux users, because files modes behave a bit differently in the specific way we're using it. It seems to come from https://github.com/golang/go/issues/60181, which is invoked because we are not using the correct file modes (because we are copying from the file we opened from writing to the file we opened for reading 😖).

It's a bit surprising that this continued to work, but the reason is essentially that all it was doing was appending the empty tempfile to ~/.aws/config, which has minimal effect, then

How did you test it?

Gave this code to a linux user who was having exceptions. This code eliminated the exceptions. Also verified through a stub program:

package main

import (
    "fmt"
    "io"
    "os"
)

func main() {
    wd, _ := os.Getwd()
    from := fmt.Sprintf("%s%c%s", wd, os.PathSeparator, "a.txt")
    to := fmt.Sprintf("%s%c%s", wd, os.PathSeparator, "b.txt")

    copyFrom, err := os.Open(from)
    fmt.Println(copyFrom)
    if err != nil {
        panic(err)
    }
    defer copyFrom.Close()
    copyTo, err := os.Create(to)
    if err != nil {
        panic(err)
    }
    defer copyTo.Close()

    b, err := io.Copy(copyTo, copyFrom)

    fmt.Printf("Copied %d bytes from %s to %s\n", b, from, to)
    if err != nil {
        panic(err)
    }
}

Populate a.txt. When using the incorrect order (io.Copy(copyTo, copyFrom)), b.txt is empty when complete. When using the correct order, it is populated. This is the desired behavior in code.

Potential risks

N/A. This is a pure fix.

Is patch release candidate?

Yes.

Link to relevant docs PRs

sosheskaz commented 6 months ago

Duplicate of #588