HTTP 400 Bad Request when calling UploadProjectMarkdown

Symptom

When using the UploadProjectMarkdown API method, the request fails with HTTP 400:

b := strings.NewReader("xxx")
_, res, err := client.ProjectMarkdownUploads.UploadProjectMarkdown(pid, b)
if err != nil {
	return fmt.Errorf("upload attachment: %w", err)
}
if res.StatusCode != http.StatusCreated {
	return fmt.Errorf("unexpected status code: %d", res.StatusCode)
}

error: upload attachment: POST https://gitlab.com/api/v4/projects/1/uploads: 400 failed to parse unknown error format: Bad Request
exit status 1

It took some digging to figure that it requires multipart upload so I tried to formulate the request myself, but the same error still persisted:

var b bytes.Buffer
w := multipart.NewWriter(&b)
p, err := w.CreateFormFile("file", "dk.png")
if err != nil {
	return fmt.Errorf("create form field: %w", err)
}
if _, err := p.Write([]byte("xxx")); err != nil {
	return fmt.Errorf("write data: %w", err)
}
w.Close()
_, res, err := client.ProjectMarkdownUploads.UploadProjectMarkdown(pid, &b, gitlab.WithHeader("Content-Type", w.FormDataContentType()))
if err != nil {
	return fmt.Errorf("upload attachment: %w", err)
}
if res.StatusCode != http.StatusCreated {
	return fmt.Errorf("unexpected status code: %d", res.StatusCode)
}

error: upload attachment: POST https://gitlab.com/api/v4/projects/1/uploads: 400 failed to parse unknown error format: Bad Request
exit status 1

Investigation

Looking at the code, it appears that there is a typo in project_markdown_uploads.go:

// We need to create the request as a GET request to make sure the options
// are set correctly. After the request is created we will overwrite both
// the method and the body.
req, err := s.client.NewRequest(http.MethodPost, u, nil, options)
if err != nil {
	return nil, nil, err
}

// Overwrite the method and body.
req.Method = http.MethodPost
req.SetBody(content)

Since the request was created as http.MethodPost, the Content-Type headers required for multipart upload cannot be passed to the underlying http.Request, thus the Content-Type remains as application/json and a HTTP 400 response was returned.

By changing http.MethodPost to http.MethodGet, the multipart headers were correctly passed to the underlying http.Request and I was able to resolve the issue:

- req, err := s.client.NewRequest(http.MethodPost, u, nil, options)
+ req, err := s.client.NewRequest(http.MethodGet, u, nil, options)
if err != nil {
	return nil, nil, err
}

Proposed Solution

To fix this issue in the codebase, I am wondering if the solution should have been to implement UploadProjectMarkdown differently?

Looking at the codebase, I observed that UploadFile uses the UploadRequest, which transparently handles the multipart upload logic that callers would otherwise have to do. If we were to change this, the UploadProjectMarkdown will require an additional filename argument.

I eventually also found out that UploadFile and UploadProjectMarkdown calls the same exact API /api/v4/projects/1/uploads. 😅

Considering the above, what is the best way to proceed with the fix?

• Was UploadFile method moved to UploadProjectMarkdown since it was no longer found on the Projects API docs? In that case, should we also update the Go client to reflect this?
• If we would like to keep UploadFile, can we reuse the same implementation for UploadProjectMarkdown? This will be a breaking change but I think it is the correct approach.