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.

Sign up for GitHub

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

Jump to bottom

ipfs add: to-files with wrap creates literal single-character dot folders #10611

Closed
3 tasks done
ProximaNova opened this issue Dec 3, 2024 · 1 comment · Fixed by #10612
Closed
3 tasks done

ipfs add: to-files with wrap creates literal single-character dot folders #10611

ProximaNova opened this issue Dec 3, 2024 · 1 comment · Fixed by #10612
Assignees
@hsanjuan
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization

Comments

@ProximaNova
Copy link

ProximaNova commented Dec 3, 2024 •
edited
Loading

Checklist

Installation method

dist.ipfs.tech or ipfs-update

Version

Kubo version: 0.32.1
Repo version: 16
System version: amd64/linux
Golang version: go1.23.3

Config

{
  "API": {
    "HTTPHeaders": {
      "Access-Control-Allow-Methods": [
        "PUT",
        "POST",
        "GET"
      ],
      "Access-Control-Allow-Origin": [
        "http://10.0.0.231:5001",
        "http://localhost:3000",
        "http://127.0.0.1:5001",
        "https://webui.ipfs.io"
      ]
    }
  },
  "Addresses": {
    "API": "/ip4/10.0.0.231/tcp/5001",
    "Announce": null,
    "AppendAnnounce": null,
    "Gateway": "/ip4/10.0.0.231/tcp/8080",
    "NoAnnounce": null,
    "Swarm": [
      "/ip4/0.0.0.0/tcp/4001",
      "/ip6/::/tcp/4001",
      "/ip4/0.0.0.0/udp/4001/webrtc-direct",
      "/ip4/0.0.0.0/udp/4001/quic-v1",
      "/ip4/0.0.0.0/udp/4001/quic-v1/webtransport",
      "/ip6/::/udp/4001/webrtc-direct",
      "/ip6/::/udp/4001/quic-v1",
      "/ip6/::/udp/4001/quic-v1/webtransport"
    ]
  },
  "AutoNAT": {},
  "Bootstrap": [
    "/dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN",
    "/dnsaddr/bootstrap.libp2p.io/p2p/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u19uLTa",
    "/dnsaddr/bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb",
    "/dnsaddr/bootstrap.libp2p.io/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt",
    "/ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ",
    "/ip4/104.131.131.82/udp/4001/quic-v1/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"
  ],
  "DNS": {
    "Resolvers": {}
  },
  "Datastore": {
    "BloomFilterSize": 0,
    "GCPeriod": "99999999999999999999h",
    "HashOnRead": false,
    "Spec": {
      "mounts": [
        {
          "child": {
            "path": "blocks",
            "shardFunc": "/repo/flatfs/shard/v1/next-to-last/2",
            "sync": true,
            "type": "flatfs"
          },
          "mountpoint": "/blocks",
          "prefix": "flatfs.datastore",
          "type": "measure"
        },
        {
          "child": {
            "compression": "none",
            "path": "datastore",
            "type": "levelds"
          },
          "mountpoint": "/",
          "prefix": "leveldb.datastore",
          "type": "measure"
        }
      ],
      "type": "mount"
    },
    "StorageGCWatermark": 908070605040302000,
    "StorageMax": "12345678910123456789GB"
  },
  "Discovery": {
    "MDNS": {
      "Enabled": true
    }
  },
  "Experimental": {
    "FilestoreEnabled": false,
    "GraphsyncEnabled": false,
    "Libp2pStreamMounting": false,
    "OptimisticProvide": false,
    "OptimisticProvideJobsPoolSize": 0,
    "P2pHttpProxy": false,
    "StrategicProviding": true,
    "UrlstoreEnabled": false
  },
  "Gateway": {
    "APICommands": [],
    "DeserializedResponses": null,
    "DisableHTMLErrors": null,
    "ExposeRoutingAPI": null,
    "HTTPHeaders": {},
    "NoDNSLink": false,
    "NoFetch": true,
    "PathPrefixes": [],
    "PublicGateways": null,
    "RootRedirect": ""
  },
  "Identity": {
    "PeerID": "12D3KooWLowyP9yZLaxzactf2WAhR6cT1nahBmeRM23zPQR3CP4T"
  },
  "Internal": {},
  "Ipns": {
    "RecordLifetime": "",
    "RepublishPeriod": "",
    "ResolveCacheSize": 128
  },
  "Ipwb": {
    "Replay": {
      "Host": "localhost",
      "Index": "/ipfs/bafkreicddpbk7q3kgranv3kchhijt24kf6f2tsow37uk34x2bu7mrfzghu",
      "Port": 2016
    }
  },
  "Migration": {
    "DownloadSources": [],
    "Keep": ""
  },
  "Mounts": {
    "FuseAllowOther": true,
    "IPFS": "/ipfs",
    "IPNS": "/ipns"
  },
  "Peering": {
    "Peers": null
  },
  "Pinning": {
    "RemoteServices": {}
  },
  "Plugins": {
    "Plugins": null
  },
  "Provider": {
    "Strategy": ""
  },
  "Pubsub": {
    "DisableSigning": false,
    "Router": ""
  },
  "Reprovider": {
    "Interval": "0"
  },
  "Routing": {
    "AcceleratedDHTClient": false,
    "Methods": null,
    "Routers": null
  },
  "Swarm": {
    "AddrFilters": null,
    "ConnMgr": {},
    "DisableBandwidthMetrics": false,
    "DisableNatPortMap": false,
    "RelayClient": {},
    "RelayService": {},
    "ResourceMgr": {},
    "Transports": {
      "Multiplexers": {},
      "Network": {},
      "Security": {}
    }
  }
}

Description

ipfs add --help does not indicate that these two options don't work so well together:
-w, --wrap-with-directory : bool - Wrap files with a directory object.
--to-files : string - Add reference to Files API (MFS) at the provided path.

Background: say that with every ipfs add command that I run, I want to add the file or folder to MFS. A month ago I added folder "a" (... --to-files=/tf/0/ ...) then today I add a different folder named "a". This is a conflict because an entry for "/tf/0/a" already exists. One would think that the solution is to only add CIDs with to-files+wrap; the expected outcome would be that that would copy the final wrap-with-directory CID to MFS. Instead what you get is...

Bug: it writes a folder literally named "." to MFS. Dealing with paths in MFS, a gateway, a WebUI, or IPFS = each does not interpret "." literally ("." and ".." are both valid linknames in IPLD/IPFS/UnixFS). So you can't do this, for example:
$ ipfs ls -s /ipfs/QmXiw5bwsuKVYEwDwLTZTu71eM43gqTGngQHFFV4pja9R5/weird/./a/
instead you have to find the CID of "." by looking in folder "weird".

I did two tests, second test:

$ ipfs add -rHw --cid-version=1 --chunker=size-1048576 --to-files=/tf/ "/home/u/Downloads/a/R"
[2 files]
added bafybeia25pwibtmoioab5apa65w6sgfnr54ranjjeg5j4ixa7gflnrueb4 R
added bafybeiakjgbiwmv24bxwjawgg3non235rlbprrctk24l6hmoi6aacw6qsy 
[===] 100.00%
$ # 1.35-GiB folder in mfs: /ipfs/QmaeewH1ZmmQwLNNC2pQ12TNf5sqRkqQ9UMdSi9WD9gyXB

The --to-files option takes the name of the final CID. It doesn't account for the case where the final CID has no name (like with -w). I think this is a bug because I don't think this is the intended or expected outcome. (I searched the issues for '"to-files" "wrap"' and saw nothing.)

BTW, if you didn't know, "." non-literally means "current directory" in various systems, so if you run "ls ." in GNU/Linux it will show you the contents of the current working directory. More helpful example: "stat ./-a.txt" vs "stat -a.txt". With this bug, a folder named . shows up as "." in ipfs, MFS, and gateway(s); it shows up as "[name of current folder]" in WebUI at http://127.0.0.1:5001/webui -> ... -> http://127.0.0.1:5001/ipfs/bafybeibgic2ex3fvzkinhy6k6aqyv3zy2o7bkbsmrzvzka24xetv7eeadm/#/files/path/to/singleDotDir (literal directory "." is falsely named "singleDotDir").
@ProximaNova ProximaNova added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Dec 3, 2024
@hsanjuan hsanjuan self-assigned this Dec 3, 2024
hsanjuan added a commit that referenced this issue Dec 3, 2024
@hsanjuan
Add: disallow --wrap with --to-files
9378dbb 
Fixes #10611.

When we are adding to an MFS folder ("--to-files=folder/"), we append the filename to the folder name.

However, when wrapping, the wrapping folder has no name and `path.Base("")` returns `.`.

This creates a folder named "." inside the previous one. But mfs operations like `ls` think that "." is the current folder, and does not contemplate the option of a folder of name ".".

Dealing with unnamed files in MFS is in general a footgun, so this commits attempts to prohibit that case.
@hsanjuan hsanjuan mentioned this issue Dec 3, 2024
Merged
hsanjuan added a commit that referenced this issue Dec 3, 2024
@hsanjuan
Add: disallow --wrap with --to-files
b320438 
Fixes #10611.

When we are adding to an MFS folder ("--to-files=folder/"), we append the filename to the folder name.

However, when wrapping, the wrapping folder has no name and `path.Base("")` returns `.`.

This creates a folder named "." inside the previous one. But mfs operations like `ls` think that "." is the current folder, and does not contemplate the option of a folder of name ".".

Dealing with unnamed files in MFS is in general a footgun, so this commits attempts to prohibit that case.
@hsanjuan
Copy link
Contributor

hsanjuan commented Dec 3, 2024

I sent a fix at #10612. I think to-files should not allow empty-named files in general as it leads to these kinds of things.

@lidel lidel closed this as completed in 433444b Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hsanjuan hsanjuan

Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cmds/add): disallow --wrap with --to-files ipfs/kubo
2 participants
@hsanjuan @ProximaNova