fix(s3): handle content types more gracefully (#3062)

## Problem
* Uploading files from the file editor overwrites the existing content type
    * Editing files without extension on S3 changes the Metadata of the object #3055
* Non-text files without a file extension throw an error

## Solution
* Shift the ContentType parameter up so callers have to be explicit
* Treat files with no mime type as binary files

I also refactored fileViewerManager.ts a bit so it could be tested with significantly less mocks

Author: JadenSimon

.changes/next-release/Bug Fix-7b9e7b89-463b-444d-99f3-d3f19d921dc2.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "S3: saving files from the editor overwrites existing content types"
+}

.changes/next-release/Bug Fix-8cd5b4be-7da7-4c7b-982e-1d828f2387cc.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "S3: file editor fails on binary files with no file extension"
+}

package-lock.json

@@ -3,11 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { S3 } from 'aws-sdk'
 import * as path from 'path'
-import { statSync } from 'fs'
+import * as mime from 'mime-types'
 import * as vscode from 'vscode'
-
+import { statSync } from 'fs'
+import { S3 } from 'aws-sdk'
 import { getLogger } from '../../shared/logger'
 import { S3Node } from '../explorer/s3Nodes'
 import { Commands } from '../../shared/vscode/commands'
@@ -365,6 +365,7 @@ async function uploadWithProgress(
         key: request.key,
         content: request.fileLocation,
         progressListener,
+        contentType: mime.contentType(path.extname(request.fileLocation.fsPath)) || undefined,
     })
 
     progressListener(0)

src/s3/commands/uploadFile.ts

@@ -5,13 +5,7 @@
 
 import * as vscode from 'vscode'
 import { ChildNodePage } from '../../awsexplorer/childNodeLoader'
-import {
-    Bucket,
-    CreateFolderRequest,
-    CreateFolderResponse,
-    S3Client,
-    UploadFileRequest,
-} from '../../shared/clients/s3Client'
+import { Bucket, CreateFolderRequest, CreateFolderResponse, S3Client } from '../../shared/clients/s3Client'
 
 import { AWSResourceNode } from '../../shared/treeview/nodes/awsResourceNode'
 import { AWSTreeNodeBase } from '../../shared/treeview/nodes/awsTreeNodeBase'
@@ -91,14 +85,6 @@ export class S3BucketNode extends AWSTreeNodeBase implements AWSResourceNode, Lo
         return this.s3.createFolder(request)
     }
 
-    /**
-     * See {@link S3Client.uploadFile}.
-     */
-    public async uploadFile(request: UploadFileRequest): Promise<void> {
-        const managedUpload = await this.s3.uploadFile(request)
-        await managedUpload.promise()
-    }
-
     /**
      * See {@link S3Client.deleteBucket}.
      */

src/s3/explorer/s3BucketNode.ts

@@ -4,14 +4,7 @@
  */
 
 import * as vscode from 'vscode'
-import {
-    Bucket,
-    CreateFolderRequest,
-    CreateFolderResponse,
-    Folder,
-    S3Client,
-    UploadFileRequest,
-} from '../../shared/clients/s3Client'
+import { Bucket, CreateFolderRequest, CreateFolderResponse, Folder, S3Client } from '../../shared/clients/s3Client'
 import { AWSResourceNode } from '../../shared/treeview/nodes/awsResourceNode'
 import { AWSTreeNodeBase } from '../../shared/treeview/nodes/awsTreeNodeBase'
 import { LoadMoreNode } from '../../shared/treeview/nodes/loadMoreNode'
@@ -90,14 +83,6 @@ export class S3FolderNode extends AWSTreeNodeBase implements AWSResourceNode, Lo
         return this.s3.createFolder(request)
     }
 
-    /**
-     * See {@link S3Client.uploadFile}.
-     */
-    public async uploadFile(request: UploadFileRequest): Promise<void> {
-        const managedUpload = await this.s3.uploadFile(request)
-        await managedUpload.promise()
-    }
-
     public get arn(): string {
         return this.folder.arn
     }

src/s3/explorer/s3FolderNode.ts

@@ -16,6 +16,7 @@ import { s3FileViewerHelpUrl } from '../shared/constants'
 import { FileProvider, VirualFileSystem } from '../shared/virtualFilesystem'
 import { PromptSettings } from '../shared/settings'
 import { telemetry } from '../shared/telemetry/telemetry'
+import { ToolkitError } from '../shared/errors'
 
 export const S3_EDIT_SCHEME = 's3'
 export const S3_READ_SCHEME = 's3-readonly'
@@ -54,8 +55,9 @@ export class S3FileProvider implements FileProvider {
         const stats = await this.client.headObject({ bucketName: bucket.name, key })
 
         this.updateETag(stats.ETag)
-        this._file.sizeBytes = stats.ContentLength
+        this._file.sizeBytes = stats.ContentLength ?? this._file.sizeBytes
         this._file.lastModified = stats.LastModified
+        this._file.ContentType = stats.ContentType
     }
 
     public async read(): Promise<Uint8Array> {
@@ -88,12 +90,13 @@ export class S3FileProvider implements FileProvider {
         return telemetry.s3_uploadObject.run(async span => {
             span.record({ component: 'viewer' })
 
+            const mimeType = mime.contentType(path.extname(this._file.name)) || undefined
             const result = await this.client
                 .uploadFile({
                     content,
                     key: this._file.key,
                     bucketName: this._file.bucket.name,
-                    contentType: mime.contentType(path.extname(this._file.name)) || undefined,
+                    contentType: this._file.ContentType ?? mimeType,
                 })
                 .then(u => u.promise())
 
@@ -123,8 +126,7 @@ export class S3FileViewerManager {
         private readonly fs: VirualFileSystem,
         private readonly window: typeof vscode.window = vscode.window,
         private readonly settings = PromptSettings.instance,
-        private readonly commands: typeof vscode.commands = vscode.commands,
-        private readonly workspace: typeof vscode.workspace = vscode.workspace
+        private readonly schemes = { read: S3_READ_SCHEME, edit: S3_EDIT_SCHEME }
     ) {
         this.disposables.push(this.registerTabCleanup())
     }
@@ -141,7 +143,7 @@ export class S3FileViewerManager {
     }
 
     private registerTabCleanup(): vscode.Disposable {
-        return this.workspace.onDidCloseTextDocument(async doc => {
+        return vscode.workspace.onDidCloseTextDocument(async doc => {
             const key = this.fs.uriToKey(doc.uri)
             await this.activeTabs[key]?.dispose()
         })
@@ -155,26 +157,29 @@ export class S3FileViewerManager {
         options?: vscode.TextDocumentShowOptions
     ): Promise<vscode.TextEditor | undefined> {
         const fsPath = fileUri.fsPath
-
         await this.activeTabs[this.fs.uriToKey(fileUri)]?.dispose()
 
-        // Defer to `vscode.open` for non-text files
-        const contentType = mime.contentType(path.extname(fsPath))
-        if (contentType && mime.charset(contentType) != 'UTF-8') {
-            await this.commands.executeCommand('vscode.open', fileUri)
-            return this.window.visibleTextEditors.find(
-                e => this.fs.uriToKey(e.document.uri) === this.fs.uriToKey(fileUri)
-            )
-        }
+        try {
+            // Defer to `vscode.open` for non-text files
+            const contentType = mime.contentType(path.extname(fsPath))
+            if (!contentType || mime.charset(contentType) !== 'UTF-8') {
+                await vscode.commands.executeCommand('vscode.open', fileUri)
+                return this.window.visibleTextEditors.find(
+                    e => this.fs.uriToKey(e.document.uri) === this.fs.uriToKey(fileUri)
+                )
+            }
 
-        const document = await this.workspace.openTextDocument(fileUri)
-        return await this.window.showTextDocument(document, options)
+            const document = await vscode.workspace.openTextDocument(fileUri)
+            return await this.window.showTextDocument(document, options)
+        } catch (err) {
+            throw ToolkitError.chain(err, 'Failed to open document', { code: 'FailedToOpen' })
+        }
     }
 
     private async closeEditor(editor: vscode.TextEditor | undefined): Promise<void> {
         if (editor && !editor.document.isClosed) {
             await this.window.showTextDocument(editor.document, { preserveFocus: false })
-            await this.commands.executeCommand('workbench.action.closeActiveEditor')
+            await vscode.commands.executeCommand('workbench.action.closeActiveEditor')
         }
     }
 
@@ -192,7 +197,7 @@ export class S3FileViewerManager {
                 await this.window.showTextDocument(activeTab.editor.document)
             } else {
                 getLogger().verbose(`S3FileViewer: Reopening non-text document`)
-                await this.commands.executeCommand('vscode.open', uri)
+                await vscode.commands.executeCommand('vscode.open', uri)
             }
 
             return activeTab
@@ -209,8 +214,8 @@ export class S3FileViewerManager {
         const contentType = mime.contentType(path.extname(file.name))
         const isTextDocument = contentType && mime.charset(contentType) == 'UTF-8'
 
-        const uri = S3FileViewerManager.fileToUri(file, TabMode.Read)
-        if (await this.tryFocusTab(uri, uri.with({ scheme: S3_EDIT_SCHEME }))) {
+        const uri = this.fileToUri(file, TabMode.Read)
+        if (await this.tryFocusTab(uri, uri.with({ scheme: this.schemes.edit }))) {
             return
         }
 
@@ -224,13 +229,13 @@ export class S3FileViewerManager {
 
     /**
      * Opens the tab in edit mode with the use of an S3Tab, or shifts focus to an edit tab if any.
-     * Exiting read-only tabs are closed as they cannot be converted to edit tabs.
+     * Existing read-only tabs are closed as they cannot be converted to edit tabs.
      *
      * @param uriOrFile to be opened
      */
     public async openInEditMode(uriOrFile: vscode.Uri | S3File): Promise<void> {
-        const uri = uriOrFile instanceof vscode.Uri ? uriOrFile : S3FileViewerManager.fileToUri(uriOrFile, TabMode.Edit)
-        const activeTab = await this.tryFocusTab(uri, uri.with({ scheme: S3_READ_SCHEME }))
+        const uri = uriOrFile instanceof vscode.Uri ? uriOrFile : this.fileToUri(uriOrFile, TabMode.Edit)
+        const activeTab = await this.tryFocusTab(uri, uri.with({ scheme: this.schemes.read }))
         const file = activeTab?.file ?? uriOrFile
 
         if (activeTab?.mode === TabMode.Edit) {
@@ -254,7 +259,7 @@ export class S3FileViewerManager {
             this.fs.registerProvider(uri, provider),
             provider.onDidChange(() => {
                 // TODO: find the correct node instead of refreshing it all
-                this.commands.executeCommand('aws.refreshAwsExplorer', true)
+                vscode.commands.executeCommand('aws.refreshAwsExplorer', true)
             })
         )
     }
@@ -267,7 +272,7 @@ export class S3FileViewerManager {
             throw new CancellationError('user')
         }
 
-        const uri = S3FileViewerManager.fileToUri(file, mode)
+        const uri = this.fileToUri(file, mode)
         const key = this.fs.uriToKey(uri)
         const provider = (this.providers[key] ??= this.registerProvider(file, uri))
         const editor = await this.openEditor(uri, { preview: mode === TabMode.Read })
@@ -353,8 +358,8 @@ export class S3FileViewerManager {
         })
     }
 
-    private static fileToUri(file: S3File, mode: TabMode): vscode.Uri {
-        const scheme = mode === TabMode.Read ? S3_READ_SCHEME : S3_EDIT_SCHEME
+    private fileToUri(file: S3File, mode: TabMode): vscode.Uri {
+        const scheme = mode === TabMode.Read ? this.schemes.read : this.schemes.edit
 
         return vscode.Uri.parse(`${scheme}:`, true).with({
             path: ['', file.bucket.region, file.bucket.name, file.key].join('/'),