fix(sheets): import mislabeled .xls workbooks by sniffing content

Local .xls files that are actually OOXML (an .xlsx exported or renamed to
.xls) failed +workbook-import with a cryptic backend
"xml_version_not_support" because the CLI trusted the file name extension.

+workbook-import now sniffs the file's leading magic bytes (PK -> xlsx,
OLE2 -> xls) and passes the true extension to the drive import core via a
new optional ImportParams.FileExtension override, correcting both the
file_extension and the staged media file name (the latter avoids the
backend's "import file extension not match", code 1069910). A declared
Excel file whose bytes match neither container is rejected locally with a
prescriptive error instead of the opaque backend failure.

The drive import core gains only the neutral FileExtension override
(empty = infer from the file name, i.e. unchanged behavior for
drive +import); all Excel sniffing/correction policy lives in the sheets
shortcut.
This commit is contained in:
xiongyuanwen-byted
2026-07-11 16:10:08 +08:00
parent cc5c738487
commit 8708ae4e1c
5 changed files with 257 additions and 42 deletions

View File

@@ -54,15 +54,21 @@ type ImportParams struct {
FolderToken string
Name string
TargetToken string
// FileExtension optionally overrides the extension inferred from File's
// name. Leave empty to infer from File (the default). Callers that have
// sniffed the file's real container use this to correct a mislabeled name
// so the backend receives the true format.
FileExtension string
}
func (p ImportParams) spec() driveImportSpec {
return driveImportSpec{
FilePath: p.File,
DocType: strings.ToLower(p.DocType),
FolderToken: p.FolderToken,
Name: p.Name,
TargetToken: p.TargetToken,
FilePath: p.File,
DocType: strings.ToLower(p.DocType),
FolderToken: p.FolderToken,
Name: p.Name,
TargetToken: p.TargetToken,
EffectiveExt: strings.TrimPrefix(strings.ToLower(p.FileExtension), "."),
}
}
@@ -127,7 +133,7 @@ func RunImport(ctx context.Context, runtime *common.RuntimeContext, p ImportPara
}
// Step 1: Upload file as media
fileToken, uploadErr := uploadMediaForImport(ctx, runtime, spec.FilePath, spec.SourceFileName(), spec.DocType)
fileToken, uploadErr := uploadMediaForImport(ctx, runtime, spec)
if uploadErr != nil {
return uploadErr
}
@@ -203,14 +209,14 @@ func preflightDriveImportFile(fio fileio.FileIO, spec *driveImportSpec) (int64,
if !info.Mode().IsRegular() {
return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", spec.FilePath).WithParam("--file")
}
if err = validateDriveImportFileSize(spec.FilePath, spec.DocType, info.Size()); err != nil {
if err = validateDriveImportFileSize(spec.FileExtension(), spec.DocType, info.Size()); err != nil {
return 0, err
}
return info.Size(), nil
}
func appendDriveImportUploadDryRun(dry *common.DryRunAPI, spec driveImportSpec, fileSize int64) {
extra, err := buildImportMediaExtra(spec.FilePath, spec.DocType)
extra, err := buildImportMediaExtra(spec.FileExtension(), spec.DocType)
if err != nil {
extra = fmt.Sprintf(`{"obj_type":"%s","file_extension":"%s"}`, spec.DocType, spec.FileExtension())
}

View File

@@ -54,14 +54,39 @@ type driveImportSpec struct {
FolderToken string
Name string
TargetToken string // existing bitable token to import data into (only for type=bitable)
// EffectiveExt is a caller-supplied override for the extension otherwise
// derived from FilePath (see ImportParams.FileExtension). It lets a caller
// that has detected the file's real container correct a mislabeled name
// (e.g. an OOXML workbook saved as .xls). Empty means "trust the filename".
EffectiveExt string
}
func (s driveImportSpec) FileExtension() string {
// rawExtension is the lowercased extension taken verbatim from the file name.
func (s driveImportSpec) rawExtension() string {
return strings.TrimPrefix(strings.ToLower(filepath.Ext(s.FilePath)), ".")
}
// FileExtension is the extension the import pipeline treats as authoritative:
// the content-sniffed override when set, otherwise the file name's extension.
func (s driveImportSpec) FileExtension() string {
if s.EffectiveExt != "" {
return s.EffectiveExt
}
return s.rawExtension()
}
// SourceFileName is the name used when staging the upload media. When content
// sniffing corrected the extension, the staged name must carry the corrected
// suffix too: the import backend cross-checks the media file name's extension
// against the file_extension in the import task and rejects a mismatch with
// "import file extension not match" (code 1069910).
func (s driveImportSpec) SourceFileName() string {
return filepath.Base(s.FilePath)
base := filepath.Base(s.FilePath)
if s.EffectiveExt != "" && s.EffectiveExt != s.rawExtension() {
base = strings.TrimSuffix(base, filepath.Ext(base)) + "." + s.EffectiveExt
}
return base
}
func (s driveImportSpec) TargetFileName() string {
@@ -92,18 +117,20 @@ func (s driveImportSpec) CreateTaskBody(fileToken string) map[string]interface{}
// uploadMediaForImport uploads the source file to the temporary import media
// endpoint and returns the file token consumed by import_tasks.
func uploadMediaForImport(ctx context.Context, runtime *common.RuntimeContext, filePath, fileName, docType string) (string, error) {
func uploadMediaForImport(ctx context.Context, runtime *common.RuntimeContext, spec driveImportSpec) (string, error) {
filePath := spec.FilePath
fileName := spec.SourceFileName()
importInfo, err := runtime.FileIO().Stat(filePath)
if err != nil {
return "", driveInputStatError(err)
}
fileSize := importInfo.Size()
if err = validateDriveImportFileSize(filePath, docType, fileSize); err != nil {
if err = validateDriveImportFileSize(spec.FileExtension(), spec.DocType, fileSize); err != nil {
return "", err
}
extra, err := buildImportMediaExtra(filePath, docType)
extra, err := buildImportMediaExtra(spec.FileExtension(), spec.DocType)
if err != nil {
return "", err
}
@@ -134,12 +161,12 @@ func uploadMediaForImport(ctx context.Context, runtime *common.RuntimeContext, f
})
}
func buildImportMediaExtra(filePath, docType string) (string, error) {
func buildImportMediaExtra(ext, docType string) (string, error) {
// The import media endpoint uses extra to decide both the target native type
// and how to interpret the uploaded source file.
extraBytes, err := json.Marshal(map[string]string{
"obj_type": docType,
"file_extension": strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), "."),
"file_extension": ext,
})
if err != nil {
return "", errs.NewInternalError(errs.SubtypeUnknown, "build upload extra failed: %v", err).WithCause(err)
@@ -147,10 +174,10 @@ func buildImportMediaExtra(filePath, docType string) (string, error) {
return string(extraBytes), nil
}
func driveImportFileSizeLimit(filePath, docType string) (int64, bool) {
func driveImportFileSizeLimit(ext, docType string) (int64, bool) {
// Keep the limit mapping local to import flows so we do not widen behavior
// changes beyond drive +import.
switch strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), ".") {
switch ext {
case "docx", "doc":
return driveImport600MBFileSizeLimit, true
case "pptx":
@@ -169,13 +196,12 @@ func driveImportFileSizeLimit(filePath, docType string) (int64, bool) {
}
}
func validateDriveImportFileSize(filePath, docType string, fileSize int64) error {
limit, ok := driveImportFileSizeLimit(filePath, docType)
func validateDriveImportFileSize(ext, docType string, fileSize int64) error {
limit, ok := driveImportFileSizeLimit(ext, docType)
if !ok || fileSize <= limit {
return nil
}
ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), ".")
if ext == "csv" {
// CSV is the only source format whose limit depends on the target type.
return errs.NewValidationError(errs.SubtypeInvalidArgument,

View File

@@ -93,61 +93,61 @@ func TestValidateDriveImportFileSize(t *testing.T) {
tests := []struct {
name string
filePath string
ext string
docType string
fileSize int64
wantText string
}{
{
name: "docx exceeds 600mb limit",
filePath: "./report.docx",
ext: "docx",
docType: "docx",
fileSize: driveImport600MBFileSizeLimit + 1,
wantText: "exceeds 600.0 MB import limit for .docx",
},
{
name: "csv sheet exceeds 20mb limit",
filePath: "./data.csv",
ext: "csv",
docType: "sheet",
fileSize: driveImport20MBFileSizeLimit + 1,
wantText: "exceeds 20.0 MB import limit for .csv when importing as sheet",
},
{
name: "csv bitable exceeds 100mb limit",
filePath: "./data.csv",
ext: "csv",
docType: "bitable",
fileSize: driveImport100MBFileSizeLimit + 1,
wantText: "exceeds 100.0 MB import limit for .csv when importing as bitable",
},
{
name: "xlsx within 800mb limit",
filePath: "./data.xlsx",
ext: "xlsx",
docType: "sheet",
fileSize: driveImport800MBFileSizeLimit,
},
{
name: "pptx exceeds 500mb limit",
filePath: "./deck.pptx",
ext: "pptx",
docType: "slides",
fileSize: driveImport500MBFileSizeLimit + 1,
wantText: "exceeds 500.0 MB import limit for .pptx",
},
{
name: "pptx within 500mb limit",
filePath: "./deck.pptx",
ext: "pptx",
docType: "slides",
fileSize: driveImport500MBFileSizeLimit,
},
{
name: "base exceeds 20mb limit",
filePath: "./snapshot.base",
ext: "base",
docType: "bitable",
fileSize: driveImport20MBFileSizeLimit + 1,
wantText: "exceeds 20.0 MB import limit for .base",
},
{
name: "base within 20mb limit",
filePath: "./snapshot.base",
ext: "base",
docType: "bitable",
fileSize: driveImport20MBFileSizeLimit,
},
@@ -157,7 +157,7 @@ func TestValidateDriveImportFileSize(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := validateDriveImportFileSize(tt.filePath, tt.docType, tt.fileSize)
err := validateDriveImportFileSize(tt.ext, tt.docType, tt.fileSize)
if tt.wantText == "" {
if err != nil {
t.Fatalf("expected no error, got %v", err)

View File

@@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"path/filepath"
"strings"
@@ -1816,9 +1817,13 @@ func lookupFirstSheetID(ctx context.Context, runtime *common.RuntimeContext, tok
//
// Imports a local xlsx/xls/csv file as a brand-new spreadsheet. The full
// upload → create-task → poll flow is the shared drive import core
// (drive.RunImport); this shortcut only pins the target type to "sheet" and
// omits the bitable-only --target-token. Symmetric with +workbook-export.
// Not exposed as an MCP tool.
// (drive.RunImport); this shortcut only pins the target type to "sheet",
// omits the bitable-only --target-token, and — because spreadsheet source
// files are routinely misnamed (an .xlsx exported/renamed to .xls, etc.) —
// sniffs the file's real container so the drive import backend receives the
// true file_extension instead of failing with a cryptic
// "xml_version_not_support". Symmetric with +workbook-export. Not exposed as
// an MCP tool.
// WorkbookImport imports a local spreadsheet file as a new Feishu spreadsheet
// by delegating to the shared drive import core with type fixed to "sheet".
@@ -1832,24 +1837,119 @@ var WorkbookImport = common.Shortcut{
HasFormat: true,
Flags: flagsFor("+workbook-import"),
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
return drive.ValidateImport(workbookImportParams(runtime))
params, err := workbookImportParams(runtime)
if err != nil {
return err
}
return drive.ValidateImport(params)
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
return drive.PlanImportDryRun(runtime, workbookImportParams(runtime))
params, err := workbookImportParams(runtime)
if err != nil {
return common.NewDryRunAPI().Set("error", err.Error())
}
dry := drive.PlanImportDryRun(runtime, params)
if note := workbookImportMislabelNote(params); note != "" {
dry.Desc(note)
}
return dry
},
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
return drive.RunImport(ctx, runtime, workbookImportParams(runtime))
params, err := workbookImportParams(runtime)
if err != nil {
return err
}
if note := workbookImportMislabelNote(params); note != "" {
fmt.Fprintln(runtime.IO().ErrOut, note)
}
return drive.RunImport(ctx, runtime, params)
},
}
// workbookImportParams builds the drive import request for +workbook-import,
// pinning DocType to "sheet". The bitable-only --target-token is intentionally
// not exposed here — use drive +import for non-sheet import targets.
func workbookImportParams(runtime *common.RuntimeContext) drive.ImportParams {
return drive.ImportParams{
File: runtime.Str("file"),
// not exposed here — use drive +import for non-sheet import targets. It also
// resolves a corrected file extension via content sniffing (see
// correctedWorkbookExtension) and surfaces it through ImportParams.FileExtension.
func workbookImportParams(runtime *common.RuntimeContext) (drive.ImportParams, error) {
file := runtime.Str("file")
params := drive.ImportParams{
File: file,
DocType: "sheet",
FolderToken: runtime.Str("folder-token"),
Name: runtime.Str("name"),
}
ext, err := correctedWorkbookExtension(runtime.FileIO(), file)
if err != nil {
return params, err
}
params.FileExtension = ext
return params, nil
}
// correctedWorkbookExtension returns an override extension when the file's
// declared .xls/.xlsx suffix disagrees with its real container, "" when the
// declared suffix is correct (or the extension is not in the Excel family, or
// the file cannot yet be read). A declared Excel file whose bytes match neither
// container yields a prescriptive validation error rather than deferring to the
// backend's opaque "xml_version_not_support".
func correctedWorkbookExtension(fio fileio.FileIO, filePath string) (string, error) {
declared := strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), ".")
if declared != "xls" && declared != "xlsx" {
return "", nil
}
sniffed, ok := sniffWorkbookContainer(fio, filePath)
if !ok {
// Not readable here; let the drive core's stat/upload surface any error.
return "", nil
}
switch sniffed {
case declared:
return "", nil
case "xls", "xlsx":
return sniffed, nil
default:
return "", errs.NewValidationError(errs.SubtypeInvalidArgument,
"file %s has a .%s extension but its content is neither an OOXML (.xlsx) nor a legacy Excel (.xls) workbook; re-save it as a real .xlsx/.xls (or export to .csv) before importing",
filePath, declared).WithParam("--file")
}
}
// sniffWorkbookContainer inspects a file's leading magic bytes to tell an OOXML
// workbook (zip container -> .xlsx) apart from a legacy OLE2/BIFF workbook
// (compound document -> .xls). The second return value is false when the file
// cannot be read far enough to judge (open error or fewer than the four
// discriminating bytes). When true, the format is "xlsx", "xls", or "" (bytes
// matching neither container).
func sniffWorkbookContainer(fio fileio.FileIO, filePath string) (string, bool) {
f, err := fio.Open(filePath)
if err != nil {
return "", false
}
defer f.Close()
var head [8]byte
n, _ := io.ReadFull(f, head[:])
if n < 4 {
return "", false
}
switch {
case head[0] == 0x50 && head[1] == 0x4B: // "PK" -> ZIP, i.e. OOXML .xlsx
return "xlsx", true
case head[0] == 0xD0 && head[1] == 0xCF && head[2] == 0x11 && head[3] == 0xE0: // OLE2 compound doc -> legacy .xls
return "xls", true
}
return "", true
}
// workbookImportMislabelNote returns a user-facing note when content sniffing
// overrode the declared extension, or "" when no correction was applied.
func workbookImportMislabelNote(params drive.ImportParams) string {
declared := strings.TrimPrefix(strings.ToLower(filepath.Ext(params.File)), ".")
if params.FileExtension == "" || params.FileExtension == declared {
return ""
}
return fmt.Sprintf("Note: %s has a mislabeled .%s extension but is actually a .%s workbook; importing it as .%s.",
filepath.Base(params.File), declared, params.FileExtension, params.FileExtension)
}

View File

@@ -10,7 +10,7 @@ import (
"testing"
"github.com/larksuite/cli/internal/httpmock"
_ "github.com/larksuite/cli/internal/vfs/localfileio"
"github.com/larksuite/cli/internal/vfs/localfileio"
)
// chdirTemp switches into a fresh temp dir for the duration of the test and
@@ -32,7 +32,7 @@ func chdirTemp(t *testing.T) {
// shared drive import core and hard-codes the import target type to "sheet".
func TestWorkbookImport_DryRunPinsSheetType(t *testing.T) {
chdirTemp(t)
if err := os.WriteFile("data.xlsx", []byte("fake-xlsx"), 0o644); err != nil {
if err := os.WriteFile("data.xlsx", []byte("PK\x03\x04fake-xlsx"), 0o644); err != nil {
t.Fatalf("write file: %v", err)
}
@@ -70,6 +70,89 @@ func TestWorkbookImport_RejectsNonSheetFile(t *testing.T) {
requireValidation(t, err, "can only be imported")
}
// TestCorrectedWorkbookExtension covers the content-sniffing that corrects (or
// rejects) a mislabeled Excel file before its extension reaches the backend.
func TestCorrectedWorkbookExtension(t *testing.T) {
ooxml := []byte("PK\x03\x04rest") // zip -> .xlsx
ole2 := []byte("\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1rest") // compound doc -> .xls
tests := []struct {
name string
fileName string
content []byte
wantExt string // expected override ("" == leave the declared extension)
wantErrSub string // non-empty == expect a validation error containing this
}{
{name: "xlsx content mislabeled as xls corrects to xlsx", fileName: "book.xls", content: ooxml, wantExt: "xlsx"},
{name: "xls content mislabeled as xlsx corrects to xls", fileName: "book.xlsx", content: ole2, wantExt: "xls"},
{name: "genuine xlsx left untouched", fileName: "book.xlsx", content: ooxml, wantExt: ""},
{name: "genuine xls left untouched", fileName: "book.xls", content: ole2, wantExt: ""},
{name: "unrecognized content on .xls rejected", fileName: "book.xls", content: []byte("<html><table></table></html>"), wantErrSub: "neither an OOXML"},
{name: "non-excel extension never sniffed", fileName: "notes.csv", content: []byte("a,b\n1,2\n"), wantExt: ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
chdirTemp(t)
if err := os.WriteFile(tt.fileName, tt.content, 0o644); err != nil {
t.Fatalf("write file: %v", err)
}
ext, err := correctedWorkbookExtension(&localfileio.LocalFileIO{}, "./"+tt.fileName)
if tt.wantErrSub != "" {
if err == nil || !strings.Contains(err.Error(), tt.wantErrSub) {
t.Fatalf("error = %v, want substring %q", err, tt.wantErrSub)
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if ext != tt.wantExt {
t.Fatalf("override ext = %q, want %q", ext, tt.wantExt)
}
})
}
}
// TestWorkbookImport_DryRunCorrectsMislabeledXls verifies an .xls file whose
// bytes are actually OOXML is imported with file_extension=xlsx end to end.
func TestWorkbookImport_DryRunCorrectsMislabeledXls(t *testing.T) {
chdirTemp(t)
if err := os.WriteFile("book.xls", []byte("PK\x03\x04zip-body"), 0o644); err != nil {
t.Fatalf("write file: %v", err)
}
calls := parseDryRunAPI(t, WorkbookImport, []string{"--file", "./book.xls"})
var createBody map[string]interface{}
for _, c := range calls {
cm, _ := c.(map[string]interface{})
if u, _ := cm["url"].(string); u == "/open-apis/drive/v1/import_tasks" {
createBody, _ = cm["body"].(map[string]interface{})
}
}
if createBody == nil {
t.Fatalf("no import_tasks create call in dry-run: %#v", calls)
}
if createBody["file_extension"] != "xlsx" {
t.Errorf("file_extension = %v, want xlsx (mislabeled .xls must be corrected)", createBody["file_extension"])
}
}
// TestWorkbookImport_RejectsUnrecognizedExcel ensures a file whose .xls/.xlsx
// name matches neither Excel container is rejected locally with a prescriptive
// error rather than deferring to the backend's opaque failure.
func TestWorkbookImport_RejectsUnrecognizedExcel(t *testing.T) {
chdirTemp(t)
if err := os.WriteFile("bogus.xls", []byte("<html>not excel</html>"), 0o644); err != nil {
t.Fatalf("write file: %v", err)
}
_, _, err := runShortcutCapturingErr(t, WorkbookImport, []string{"--file", "./bogus.xls", "--dry-run"})
requireValidation(t, err, "neither an OOXML")
}
// TestWorkbookImport_ExecuteCreatesSheet runs the full upload → create → poll
// flow against stubs and asserts the resulting URL is a /sheets/ link.
func TestWorkbookImport_ExecuteCreatesSheet(t *testing.T) {