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

chore: revert back to FileServer returning an http handler #86

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"github.com/coder/code-marketplace/api/httpapi"
"github.com/coder/code-marketplace/api/httpmw"
"github.com/coder/code-marketplace/database"
"github.com/coder/code-marketplace/storage"

Check failure on line 16 in api/api.go

View workflow job for this annotation

GitHub Actions / lint

could not import github.com/coder/code-marketplace/storage (-: # github.com/coder/code-marketplace/storage
)

const MaxPageSizeDefault int = 200
Expand Down Expand Up @@ -112,7 +112,7 @@
r.Post("/api/extensionquery", api.extensionQuery)

// Endpoint for getting an extension's files or the extension zip.
r.Mount("/files", http.StripPrefix("/files", storage.HTTPFileServer(options.Storage)))
r.Mount("/files", http.StripPrefix("/files", options.Storage.FileServer()))

// VS Code can use the files in the response to get file paths but it will
// sometimes ignore that and use requests to /assets with hardcoded types to
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/go-chi/httprate v0.14.1
github.com/google/uuid v1.6.0
github.com/lithammer/fuzzysearch v1.1.8
github.com/spf13/afero v1.11.0
github.com/spf13/cobra v1.8.1
github.com/stretchr/testify v1.9.0
golang.org/x/mod v0.19.0
Expand All @@ -18,6 +17,7 @@ require (
)

require (
cloud.google.com/go/logging v1.8.1 // indirect
github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/charmbracelet/lipgloss v0.7.1 // indirect
Expand All @@ -38,6 +38,9 @@ require (
golang.org/x/sys v0.17.0 // indirect
golang.org/x/term v0.17.0 // indirect
golang.org/x/text v0.14.0 // indirect
google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20231106174013-bbf56f31fb17 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20231120223509-83a465c0220f // indirect
google.golang.org/protobuf v1.32.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
10 changes: 4 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ cloud.google.com/go/compute v1.23.3 h1:6sVlXXBmbd7jNX0Ipq0trII3e4n1/MsADLK6a+aiV
cloud.google.com/go/compute v1.23.3/go.mod h1:VCgBUoMnIVIR0CscqQiPJLAG25E3ZRZMzcFZeQ+h8CI=
cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY=
cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA=
cloud.google.com/go/logging v1.7.0 h1:CJYxlNNNNAMkHp9em/YEXcfJg+rPDg7YfwoRpMU+t5I=
cloud.google.com/go/logging v1.7.0/go.mod h1:3xjP2CjkM3ZkO73aj4ASA5wRPGGCRrPIAeNqVNkzY8M=
cloud.google.com/go/longrunning v0.5.1 h1:Fr7TXftcqTudoyRJa113hyaqlGdiBQkp0Gq7tErFDWI=
cloud.google.com/go/longrunning v0.5.1/go.mod h1:spvimkwdz6SPWKEt/XBij79E9fiTkHSQl/fRUUQJYJc=
cloud.google.com/go/logging v1.8.1 h1:26skQWPeYhvIasWKm48+Eq7oUqdcdbwsCVwz5Ys0FvU=
cloud.google.com/go/logging v1.8.1/go.mod h1:TJjR+SimHwuC8MZ9cjByQulAMgni+RkXeI3wwctHJEI=
cloud.google.com/go/longrunning v0.5.4 h1:w8xEcbZodnA2BbW6sVirkkoC+1gP8wS57EUUgGS0GVg=
cloud.google.com/go/longrunning v0.5.4/go.mod h1:zqNVncI0BOP8ST6XQD1+VcvuShMmq7+xFSzOL++V0dI=
github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k=
github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
Expand Down Expand Up @@ -56,8 +56,6 @@ github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJ
github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis=
github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8=
github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY=
github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
Expand Down
50 changes: 18 additions & 32 deletions storage/artifactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"net/http"
"os"
"path"
Expand All @@ -16,7 +15,6 @@ import (
"sync"
"time"

"github.com/spf13/afero/mem"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"

Expand Down Expand Up @@ -277,37 +275,25 @@ func (s *Artifactory) AddExtension(ctx context.Context, manifest *VSIXManifest,
return s.uri + dir, nil
}

// Open returns a file from Artifactory.
// TODO: Since we only extract a subset of files perhaps if the file does not
// exist we should download the vsix and extract the requested file as a
// fallback. Obviously this seems like quite a bit of overhead so we would
// then emit a warning so we can notice that VS Code has added new asset types
// that we should be extracting to avoid that overhead. Other solutions could
// be implemented though like extracting the VSIX to disk locally and only
// going to Artifactory for the VSIX when it is missing on disk (basically
// using the disk as a cache).
func (s *Artifactory) Open(ctx context.Context, fp string) (fs.File, error) {
resp, code, err := s.read(ctx, fp)
if code != http.StatusOK || err != nil {
switch code {
case http.StatusNotFound:
return nil, fs.ErrNotExist
case http.StatusForbidden:
return nil, fs.ErrPermission
default:
return nil, err
func (s *Artifactory) FileServer() http.Handler {
// TODO: Since we only extract a subset of files perhaps if the file does not
// exist we should download the vsix and extract the requested file as a
// fallback. Obviously this seems like quite a bit of overhead so we would
// then emit a warning so we can notice that VS Code has added new asset types
// that we should be extracting to avoid that overhead. Other solutions could
// be implemented though like extracting the VSIX to disk locally and only
// going to Artifactory for the VSIX when it is missing on disk (basically
// using the disk as a cache).
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
reader, code, err := s.read(r.Context(), r.URL.Path)
if err != nil {
http.Error(rw, err.Error(), code)
return
}
}

// TODO: Do no copy the bytes into memory, stream them rather than
// storing the entire file into memory.
f := mem.NewFileHandle(mem.CreateFile(fp))
_, err = io.Copy(f, resp)
if err != nil {
return nil, err
}

return f, nil
defer reader.Close()
rw.WriteHeader(http.StatusOK)
_, _ = io.Copy(rw, reader)
})
}

func (s *Artifactory) Manifest(ctx context.Context, publisher, name string, version Version) (*VSIXManifest, error) {
Expand Down
4 changes: 4 additions & 0 deletions storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ func (s *Local) AddExtension(ctx context.Context, manifest *VSIXManifest, vsix [
return dir, nil
}

func (s *Local) FileServer() http.Handler {
return http.FileServer(http.Dir(s.extdir))
}

func (s *Local) Open(_ context.Context, fp string) (fs.File, error) {
return http.Dir(s.extdir).Open(fp)
}
Expand Down
85 changes: 28 additions & 57 deletions storage/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
"context"
"crypto"
"encoding/json"
"io"
"io/fs"
"path/filepath"
"net/http"
"strconv"
"strings"

"github.com/spf13/afero/mem"
"golang.org/x/xerrors"

"cdr.dev/slog"
"github.com/coder/code-marketplace/api/httpapi"
"github.com/coder/code-marketplace/api/httpmw"

"github.com/coder/code-marketplace/extensionsign"
)
Expand Down Expand Up @@ -113,7 +113,7 @@
return manifest, nil
}

// Open will intercept requests for signed extensions payload.
// FileServer will intercept requests for signed extensions payload.
// It does this by looking for 'SigzipFileExtension' or p7s.sig.
//
// The signed payload and signing process is taken from:
Expand All @@ -125,61 +125,32 @@
// the signature. Meaning the signature could be empty, incorrect, or a
// picture of cat and it would work. There is no signature verification.
//
// - VSCode requires a signature payload to exist, but the context appear
// to be somewhat optional.
// Following another open source implementation, it appears the '.signature.p7s'
// file must exist, but it can be empty.
// The signature is stored in a '.signature.sig' file, although it is unclear
// is VSCode ever reads this file.
// TODO: Properly implement the p7s file, and diverge from the other open
// source implementation. Ideally this marketplace would match Microsoft's
// marketplace API.
func (s *Signature) Open(ctx context.Context, fp string) (fs.File, error) {
if s.SigningEnabled() && strings.HasSuffix(filepath.Base(fp), SigzipFileExtension) {
base := filepath.Base(fp)
vsixPath := strings.TrimSuffix(base, SigzipFileExtension)

// hijack this request, sign the sig manifest
manifest, err := s.Storage.Open(ctx, filepath.Join(filepath.Dir(fp), sigManifestName))
if err != nil {
// If this file is missing, it means the extension was added before
// signatures were handled by the marketplace.
// TODO: Generate the sig manifest payload and insert it?
return nil, xerrors.Errorf("open signature manifest: %w", err)
}
defer manifest.Close()

manifestData, err := io.ReadAll(manifest)
if err != nil {
return nil, xerrors.Errorf("read signature manifest: %w", err)
}

vsix, err := s.Storage.Open(ctx, filepath.Join(filepath.Dir(fp), vsixPath+".vsix"))
if err != nil {
// If this file is missing, it means the extension was added before
// signatures were handled by the marketplace.
// TODO: Generate the sig manifest payload and insert it?
return nil, xerrors.Errorf("open signature manifest: %w", err)
}
defer vsix.Close()
// - VSCode requires a signature payload to exist, but the content is optional
// for linux users.
// For windows (maybe mac?) users, the signature must be valid, and this
// implementation will not work.
func (s *Signature) FileServer() http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
if s.SigningEnabled() && strings.HasSuffix(r.URL.Path, SigzipFileExtension) {
// hijack this request, return an empty signature payload
signed, err := extensionsign.IncludeEmptySignature()

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / lint

undefined: extensionsign.IncludeEmptySignature (typecheck)

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / lint

undefined: extensionsign.IncludeEmptySignature) (typecheck)

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / lint

undefined: extensionsign.IncludeEmptySignature) (typecheck)

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / lint

undefined: extensionsign.IncludeEmptySignature) (typecheck)

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest)

undefined: extensionsign.IncludeEmptySignature

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest)

undefined: extensionsign.IncludeEmptySignature

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / test (macos-latest)

undefined: extensionsign.IncludeEmptySignature

Check failure on line 136 in storage/signature.go

View workflow job for this annotation

GitHub Actions / test (macos-latest)

undefined: extensionsign.IncludeEmptySignature
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.ErrorResponse{
Message: "Unable to generate empty signature for extension",
Detail: err.Error(),
RequestID: httpmw.RequestID(r),
})
return
}

vsixData, err := io.ReadAll(vsix)
if err != nil {
return nil, xerrors.Errorf("read signature manifest: %w", err)
rw.Header().Set("Content-Length", strconv.FormatInt(int64(len(signed)), 10))
rw.WriteHeader(http.StatusOK)
_, _ = rw.Write(signed)
return
}

// TODO: Fetch the VSIX payload from the storage
signed, err := s.SigZip(ctx, vsixData, manifestData)
if err != nil {
return nil, xerrors.Errorf("sign and zip manifest: %w", err)
}

f := mem.NewFileHandle(mem.CreateFile(fp))
_, err = f.Write(signed)
return f, err
}

return s.Storage.Open(ctx, fp)
s.Storage.FileServer().ServeHTTP(rw, r)
})
}

func (s *Signature) SigZip(ctx context.Context, vsix []byte, sigManifest []byte) ([]byte, error) {
Expand Down
18 changes: 3 additions & 15 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,9 @@ type Storage interface {
// for verification purposes. Extra files can be included, but not required.
// All extra files will be placed relative to the manifest outside the vsix.
AddExtension(ctx context.Context, manifest *VSIXManifest, vsix []byte, extra ...File) (string, error)
// Open mirrors the fs.FS interface of Open, except with a context.
// The Open should return files from the extension storage, and used for
// serving extensions.
Open(ctx context.Context, name string) (fs.File, error)
// FileServer provides a handler for fetching extension repository files from
// a client.
FileServer() http.Handler
// Manifest returns the manifest bytes for the provided extension. The
// extension asset itself (the VSIX) will be included on the manifest even if
// it does not exist on the manifest on disk.
Expand All @@ -239,17 +238,6 @@ type Storage interface {
WalkExtensions(ctx context.Context, fn func(manifest *VSIXManifest, versions []Version) error) error
}

// HTTPFileServer creates an http.Handler that serves files from the provided
// storage.
func HTTPFileServer(s Storage) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
http.FileServerFS(&contextFs{
ctx: r.Context(),
open: s.Open,
}).ServeHTTP(rw, r)
})
}

type File struct {
RelativePath string
Content []byte
Expand Down
2 changes: 1 addition & 1 deletion storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func testFileServer(t *testing.T, factory storageFactory) {
req := httptest.NewRequest("GET", test.path, nil)
rec := httptest.NewRecorder()

server := storage.HTTPFileServer(f.storage)
server := f.storage.FileServer()
server.ServeHTTP(rec, req)

resp := rec.Result()
Expand Down
13 changes: 0 additions & 13 deletions testutil/mockstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ package testutil
import (
"context"
"errors"
"io/fs"
"net/http"
"os"
"path/filepath"
"sort"

"github.com/spf13/afero/mem"

"github.com/coder/code-marketplace/storage"
)

Expand All @@ -26,15 +22,6 @@ func NewMockStorage() *MockStorage {
func (s *MockStorage) AddExtension(ctx context.Context, manifest *storage.VSIXManifest, vsix []byte, extra ...storage.File) (string, error) {
return "", errors.New("not implemented")
}
func (s *MockStorage) Open(ctx context.Context, path string) (fs.File, error) {
if filepath.Base(path) == "nonexistent" {
return nil, fs.ErrNotExist
}

f := mem.NewFileHandle(mem.CreateFile(path))
_, _ = f.Write([]byte("foobar"))
return f, nil
}

func (s *MockStorage) FileServer() http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
Expand Down
Loading