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

Created/updated binary files are not recognized as binary data #925

Closed
aaron-comyn opened this issue Oct 5, 2015 · 5 comments
Closed

Created/updated binary files are not recognized as binary data #925

aaron-comyn opened this issue Oct 5, 2015 · 5 comments
Labels
Type: Bug Something isn't working as documented

Comments

@aaron-comyn
Copy link

Unless I'm missing something obvious: it seems that binary files uploaded through CreateFile/UpdateFile methods in Octokit.net cannot be flagged as binary data and are interpreted server-side as text.

Judging from other Octokit libraries there may be an encoding option not being passed through the API when creating and updating new files (isBase64 or isBinary, more info).

The low-level API handles this properly (with TreeType.Blob and EncodingType.Base64), but it takes a lot more Git knowhow and 6 calls to the API instead of 1 :)


Examples to show the difference:

A file checked in through repository vs a file added through the API
The RepositoryContent returned through Octokit.net for both objects shows no appreciable difference.

Calling code (F#):

    let update = new UpdateFileRequest("[Automatic] update", 
                                       encodedFileContents, 
                                       targetFileInfo.Sha, 
                                       Committer = new SignatureResponse(name, email, date)

    let updatedContent =  
        async { return repo.UpdateFile(repoOwner, repoName, filePath, update).Result } 
        |> Async.RunSynchronously
@poisonous-milk
Copy link
Contributor

Cannot find a parameter in Created/Updated API that can clarify the type of the file.

@aaron-comyn
Copy link
Author

The Github API documentation makes no reference to any kind of binary parameter on CreateFile or UpdateFile, but it looks like they accept an 'encoding' parameter. Octokit.net seems to be using the default of UTF-8 even for base64 text - supposedly the same issue can be resolved in Octokit.js by setting the encoding of binary text to 'base64'.

Same issue with a pull request showing a 'working' solution. Relevant update:

               content =
                content: content,
                encoding: 'base64'

From poking around the code it looks like adding a new property to the CreateFileRequest object (CreateFileRequest.cs), that mapped to the appropriate encoding parameter (or a prop called "IsBinary" that would serialize properly), would do the trick. That parameter would propagate all the way through to the API through the ApiConnection, it's just a matter of finding the correct structure for the encoding values :)

@naveensrinivasan
Copy link

When the API dose not officially support it then why build a hack around it?

@aaron-comyn
Copy link
Author

The API does not list the "encoding" parameter in its Repository Content docs, but it accepts/returns an encoding, and uses a specific 'base64' encoding parameter elsewhere for binary operations... Encoding = EncodingType.Base64 is how it's handled on BLOBs.

As a matter of definition: arguably this is a pure API inconsistency, or a documentation oversight... the workflow of Git and GitHub allow it in single operation. Perhaps updating the docs would be better, either making the encoding parameter official, or clarifying that the RepositoryContent API doesn't handle binary data. No hacks needed ;)


Feel free to close the issue - I opened it primarily to provide a path for any other people stuck on the API inconsistency, not everyone is comfortable with the BLOB DB.

Upload of a single binary file:

let repoOwner = "me"
let repoName = "myrepo"

let github = new GitHubClient(new ProductHeaderValue("name"), Credentials = new Credentials(secrets))  
let gitdb = github.GitDatabase
let gitblobs = github.GitDatabase.Blob

let runAsyncFunc (f:Task<'a>) = f.Result
let (~%%) asyncFunc = runAsyncFunc asyncFunc

member this.Update filePath encodedFileContents =
    let file = new NewBlob(Encoding = EncodingType.Base64, Content = (encodedFileContents))
    let blob = %% gitblobs.Create(repoOwner, repoName, file)

    let master =  %% gitdb.Reference.Get(repoOwner, repoName, @"heads/master")
    let baseTree = %% gitdb.Commit.Get(repoOwner, repoName, master.Object.Sha) 

    let nt = new NewTree(BaseTree = baseTree.Sha)
    nt.Tree.Add(new NewTreeItem(Path = filePath, Mode = "100755", Type = TreeType.Blob, Sha = blob.Sha ))

    let newTree = %% gitdb.Tree.Create(repoOwner, repoName, nt)
    let newCommit = new NewCommit(@"[Automatic] metadata update of " + filePath, newTree.Sha, [ master.Object.Sha ])

    let commit =  %% gitdb.Commit.Create(repoOwner, repoName, newCommit)
    let ref = %% gitdb.Reference.Update(repoOwner, repoName, @"heads/master", new ReferenceUpdate(commit.Sha))
    ()

@shiftkey
Copy link
Member

@aaron-comyn oops, totally missed that last message.

Thanks for the details, and there's actually some things we do under the hood to take the control away from you here - the Content property is a .NET string and we decorate it with a [SerializeAsBase64] attribute so that the user doesn't need to remember to do that. That's clearly not what you're looking for here, and I'm grateful for that example using the Git Data API.

I've opened #1143 to track this feature as a parity item.

@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

5 participants