Support manifest traversal #452

zbiljic · 2020-07-16T10:38:44Z

NOTE: This is PoC, and uses simple, JSON-based manifest structure, which will be changed to solidity friendly implementation later on. It also lacks various protections against possible attacks.

Includes initial support for resolving the relative path from the bzz URL.
If the referenced address (hash) is of type manifest, it will attempt to lookup noted path from URL in that manifest, and if matched return the content of that file.

Route used is /bzz:/{address}/{path:.*}.

This implementation borrowed heavily from file.go and bytes.go. Initial PoC uses JSON-based manifest (for ease of testing), which is identified by application/bzz-manifest+json MIME type in file metadata. Manifest entry structure contains headers for the referenced file, which will be populated on response. All manifest-based operations are hidden behind interfaces, which should allow for easier switch to any other structure for manifest later on.

Example of usage for PoC website:

step 1 - upload 'index.html'

http POST http://127.0.0.1:8081/files "Content-Type:text/html" < index.html

Used HTML content:

<!doctype html>
<html>
<head>
    <title>Swarm website</title>
</head>
<body>
    <img src="images/ethersphere.png" alt="ethersphere">
    <p>Swarm: Censorship resistant storage and communication infrastructure for a truly sovereign digital society</p>
</body>
</html>

step 2 - upload some image

For example, 'ethersphere.png' was taken.

http POST http://127.0.0.1:8081/files "Content-Type:image/png" < ethersphere.png

step 3 - manually create manifest JSON

References used are the ones received for each file upload.
Note that 'index.html' uses 'ethersphere.png' on specific path.

{
    "entries": {
        "index.html": {
            "reference": "<INDEX_HTML_REFERENCE>",
            "name": "index.html",
            "headers": {
                "Content-Type": ["text/html"]
            }
        },
        "images/ethersphere.png": {
            "reference": "<PNG_REFERENCE>",
            "name": "ethersphere.png",
            "headers": {
                "Content-Type": ["image/png"]
            }
        }
    }
}

step 4 - upload created manifest

We use /files API, and set special content type for manifest file (for this PoC).

http POST http://127.0.0.1:8081/files "Content-Type:application/bzz-manifest+json" < manifest.json

step 5 - check "website"

It should now be possible to open the "website" in browser by going to:

http://127.0.0.1:8081/bzz:/<MANIFEST_FILE_REFERENCE>/index.html

For retreiving file ntries using paths.

Manually join header values, as for some reason HTTP layer uses only single value.

pradovic

Looks good to me! Left a few style-related comments.

pkg/api/bzz.go

pradovic · 2020-07-16T11:08:56Z

pkg/api/bzz_test.go

+		fileName := "sample.html"
+		filePath := "test/" + fileName
+		missingFilePath := "test/missing"
+		sampleHtml := `<!DOCTYPE html>
+		<html>
+		<body>
+
+		<h1>My First Heading</h1>
+
+		<p>My first paragraph.</p>
+
+		</body>
+		</html>`
+
+		var err error
+		var fileReference swarm.Address
+		var manifestFileReference swarm.Address


I would maybe put this in the TestBzz scope and just have these individual tests below. Don't see a need for sub-tests.

vandot · 2020-07-16T12:19:26Z

pkg/api/router.go

@@ -54,6 +54,10 @@ func (s *server) setupRouting() {
 		"POST": http.HandlerFunc(s.chunkUploadHandler),
 	})

+	handle(router, "/bzz:/{address}/{path:.*}", jsonhttp.MethodHandler{


we agreed to use /bzz:/ notation?
https://beehive.ethswarm.org/swarm/pl/fzgc13trxpgjz8k37ft3y8zzgo

https://beehive.ethswarm.org/swarm/pl/sfyot87eqin68cpsum3qspwp7w, I guess we just agreed to try /bzz/?

from these links, it is unclear to me tbh

pradovic

LGTM

zelig

looks promising, need to understand why the code is so complex

zelig · 2020-07-17T04:37:36Z

pkg/api/bzz.go

+
+	entryAddress := entry.GetReference()
+
+	toDecryptEntry := len(entryAddress.Bytes()) == (swarm.HashSize + encryption.KeyLength)


maybe we should extract this as a function on addresses

pkg/api/bzz_test.go

pkg/api/bzz.go

mortelli

LGTM, only minor remarks.

mortelli · 2020-07-17T02:52:30Z

pkg/api/router.go

@@ -54,6 +54,10 @@ func (s *server) setupRouting() {
 		"POST": http.HandlerFunc(s.chunkUploadHandler),
 	})

+	handle(router, "/bzz:/{address}/{path:.*}", jsonhttp.MethodHandler{


from these links, it is unclear to me tbh

pkg/manifest/manifest.go

pkg/api/common.go

pkg/api/bzz.go

pkg/api/bzz_test.go

nolash · 2020-07-17T16:43:22Z

pkg/api/bzz.go

+	ManifestContentType = "application/bzz-manifest+json"
+)
+
+func (s *server) bzzDownloadHandler(w http.ResponseWriter, r *http.Request) {


This is a very long function

That it is. It was intentionally left like that, to have linear steps that are being performed. There is also a lot of boilerplate regarding HTTP API.
BTW, not the longest one, the file.go wins in that regard.

well mistakes don't need to be repeated

Terribly sorry but I would say this function is not specially long, and having a "long" function is not bad in any general way. Also, we are trying to isolate the PRs to be small, and limit the change set, so I would not spend to much time here to refactor the files.go code. This function is using a code from files.go, and although I agree it should be refactored, I don't think it needs to be in this specific PR. Especially, since we asked about this on mattermost and got the info that the code inside was not considered business logic and that it should be used in this way: https://beehive.ethswarm.org/swarm/pl/83bayftmzfgqbqgocpyxwkn3xc.
In order to refactor this function we should probably:

Extract specific error types

Remove logging and request rendering form the function and move it to calling functions, based on extracted error types

Maybe move this to a special upload/download/transfer package/component in pkg and let api be thin and only API related

So, anyways, my 2 c are that we should keep this PR minimal and focus on the user-story, and open the separate PR to refactor the code, since this code will move stuff from files.go and possibly other api calls as well. Just my 2 c, I am OK either way.

mortelli · 2020-07-17T19:23:00Z

@zbiljic thank you very much for addressing all my comments 👍

mortelli · 2020-07-22T13:58:17Z

related follow-up issue: #463

zbiljic added 7 commits July 14, 2020 20:50

Add initial manifest implementation

5d131af

Include initial high level storage API

9638756

For retreiving file ntries using paths.

Create JSON implementation for manifest

76fa6c8

Inject all file header from manifest

0b641eb

Support multiple values for header

7b88eff

Manually join header values, as for some reason HTTP layer uses only single value.

Rename 'address' in manifest to 'reference'

160dbb1

User JSON manifeset implementation

13b821d

zbiljic added the in progress ongoing development , hold out with review label Jul 16, 2020

pradovic mentioned this pull request Jul 16, 2020

Support manifest traversal #446

Closed

pradovic assigned zbiljic and pradovic Jul 16, 2020

pradovic linked an issue Jul 16, 2020 that may be closed by this pull request

Support path in url and map to manifest traversal #433

Closed

pradovic reviewed Jul 16, 2020

View reviewed changes

Update few style-related comments

7f1d7bd

zbiljic requested review from pradovic, zelig and mortelli July 16, 2020 11:34

zbiljic added ready for review The PR is ready to be reviewed and removed in progress ongoing development , hold out with review labels Jul 16, 2020

vandot reviewed Jul 16, 2020

View reviewed changes

pradovic approved these changes Jul 16, 2020

View reviewed changes

mortelli mentioned this pull request Jul 16, 2020

api: upload directory #447

Merged

zelig reviewed Jul 17, 2020

View reviewed changes

zbiljic added 3 commits July 17, 2020 14:25

Extract common code for downloading file content

d3c1f96

Update style-related comments

6ba5df7

Move download handler function to existing file

5f94d6c

mortelli approved these changes Jul 17, 2020

View reviewed changes

zbiljic added 2 commits July 17, 2020 16:47

Resolve style comments

4cb1efd

Document interfaces for manifest package

af4c484

nolash reviewed Jul 17, 2020

View reviewed changes

zelig approved these changes Jul 20, 2020

View reviewed changes

Manifest references file (instead of raw bytes)

475e688

zbiljic merged commit 1467336 into master Jul 21, 2020

zbiljic deleted the feature/manifest-traversal branch July 21, 2020 13:40

zbiljic mentioned this pull request Jul 21, 2020

Fix endpoint for manifest traversal #459

Closed

This was referenced Jul 22, 2020

decouple api package #462

Closed

remove redundant entries in upload/download #463

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support manifest traversal #452

Support manifest traversal #452

zbiljic commented Jul 16, 2020 •

edited

pradovic left a comment

pradovic Jul 16, 2020

vandot Jul 16, 2020

pradovic Jul 16, 2020

mortelli Jul 17, 2020

pradovic left a comment

zelig left a comment

zelig Jul 17, 2020

mortelli left a comment

mortelli Jul 17, 2020

nolash Jul 17, 2020

zbiljic Jul 17, 2020

nolash Jul 17, 2020

pradovic Jul 18, 2020 •

edited

mortelli commented Jul 17, 2020

mortelli commented Jul 22, 2020


		entryAddress := entry.GetReference()

		toDecryptEntry := len(entryAddress.Bytes()) == (swarm.HashSize + encryption.KeyLength)

Support manifest traversal #452

Support manifest traversal #452

Conversation

zbiljic commented Jul 16, 2020 • edited

Example of usage for PoC website:

step 1 - upload 'index.html'

step 2 - upload some image

step 3 - manually create manifest JSON

step 4 - upload created manifest

step 5 - check "website"

pradovic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pradovic left a comment

Choose a reason for hiding this comment

zelig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mortelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pradovic Jul 18, 2020 • edited

Choose a reason for hiding this comment

mortelli commented Jul 17, 2020

mortelli commented Jul 22, 2020

zbiljic commented Jul 16, 2020 •

edited

pradovic Jul 18, 2020 •

edited