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

feat: localhost subdomain redirection #136

Merged
merged 11 commits into from
Sep 22, 2022
Merged

feat: localhost subdomain redirection #136

merged 11 commits into from
Sep 22, 2022

Conversation

tomicvladan
Copy link
Collaborator

@nugaon nugaon changed the title Feat/subdomain redirection feat: subdomain redirection Sep 5, 2022
nugaon
nugaon requested changes Sep 5, 2022
View reviewed changes
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work only in case of localhost as that is the only supported redirection from Bee side.

nugaon
nugaon reviewed Sep 9, 2022
View reviewed changes
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it out with my local build but it does not redirect the bzz://{content hash} to the localhost subdomain...

src/utils/bzz-link.ts Outdated
@@ -62,6 +66,22 @@ export function subdomainToBzzResource(subdomain: string): string {
return `${subdomain}.eth`
}

export function isHostIpAddress(url: string): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used function

src/utils/bzz-link.ts Outdated
@@ -1,6 +1,10 @@
/** bzz.link CID implementaion */
import * as swarmCid from '@ethersphere/swarm-cid'

const ipAsHostRegex = RegExp(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not needed anymore.

@tomicvladan
Copy link
Collaborator Author

Now it should work. Could you try again?

@nugaon
Copy link
Member

nugaon commented Sep 9, 2022

one additional thing that we missed out is updating sandboxListener in the bee-api.listener.ts.
We don't need to apply sandbox on bee responses if the subdomain redirection is used.

@tomicvladan
Copy link
Collaborator Author

I think the sandboxListener won't be triggered at all, because the URL will be different from the bee URL, because of the prefix. But that might cause issue for other listeners.

swarm-extension/src/background/listener/bee-api.listener.ts

Lines 76 to 91 in 554c9ff

private addBeeNodeListeners(beeApiUrl: string) {
chrome.webRequest.onBeforeSendHeaders.addListener(
this.globalPostageStampHeaderListener,
{
urls: [`${beeApiUrl}/*`],
},
['blocking', 'requestHeaders'],
)
chrome.webRequest.onHeadersReceived.addListener(
this.sandboxListener,
{
urls: [`${beeApiUrl}/*`],
},
['blocking', 'responseHeaders', 'extraHeaders'],
)
}

nugaon
nugaon requested changes Sep 9, 2022
View reviewed changes
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it still does not work on my side...
we need to redirect

  • bzz://{swarm hash or ens}/{path} and
  • https://{cid or ens}.bzz.link/{path}
    to http://{cid or ens without .eth}.swarm.localhost:{port}/{path}

src/utils/bzz-link.ts Outdated
export function createSubdomainUrl(beeApiUrl: string, subdomain: string): string {
const [protocol, host] = beeApiUrl.split('://')

return `${protocol}://${subdomain}.${host}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

host here should be prefixed with swarm.

@nugaon nugaon changed the title feat: subdomain redirection feat: localhost subdomain redirection Sep 9, 2022
@nugaon
Copy link
Member

nugaon commented Sep 9, 2022

now it seems like it creates a CID from a CID of the bzz.link address.
e.g. bah5acgzaunpsiqopkg7tmibpy2g3jchobzsjinftr4ps7hicxgvjstflnhtq.bzz.link -> http://a35f2441cf51bf36202fc68db488ee0e649434b38f1f2f9d02b9aa994cab69e7.swarm.localhost:1633/

@nugaon
Copy link
Member

nugaon commented Sep 12, 2022

now, the subdomain is mapped to the swarm hash reference instead of the CID. please check out the desired mapping in my previous comment

nugaon
nugaon reviewed Sep 12, 2022
View reviewed changes
src/background/listener/bee-api.listener.ts Outdated
@@ -149,7 +149,7 @@ export class BeeApiListener {
const bzzReference = subdomainToBzzResource(subdomain) + pathWithParams
console.log('bzz link redirect', bzzReference, url, pathWithParams)

this.redirectToBzzReference(bzzReference, tabId)
this.redirectToBzzReference(bzzReference, tabId, true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you do that? cid.bzz.link also should be mapped to {cid}.swarm.localhost

nugaon
nugaon reviewed Sep 12, 2022
View reviewed changes
src/background/listener/bee-api.listener.ts Outdated
Comment on lines 286 to 292
let subdomain = hash

if (subdomain.endsWith('.eth')) {
subdomain = subdomain.substring(0, subdomain.length - 4)
}

url = createSubdomainUrl(this._beeApiUrl, subdomain)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here the subdomain is a swarm hash, but u don't transform it into a CID that the swarm.localhost subdomain requires.

nugaon
nugaon reviewed Sep 12, 2022
View reviewed changes
src/background/listener/bee-api.listener.ts Outdated
@@ -276,8 +276,25 @@ export class BeeApiListener {
* @param bzzReference in form of $ROOT_HASH<$PATH><$QUERY>
* @param tabId the tab will be navigated to the dApp page
*/
private redirectToBzzReference(bzzReference: string, tabId: number) {
const url = `${this._beeApiUrl}/bzz/${bzzReference}`
private redirectToBzzReference(bzzReference: string, tabId: number, preventSubdomainRedirection = false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove `preventSubdomainRedirection

Suggested change
private redirectToBzzReference(bzzReference: string, tabId: number, preventSubdomainRedirection = false) {
private redirectToBzzReference(bzzReference: string, tabId: number) {

nugaon
nugaon approved these changes Sep 20, 2022
View reviewed changes
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tomicvladan tomicvladan merged commit 082f053 into master Sep 22, 2022
@tomicvladan tomicvladan deleted the feat/subdomain-redirection branch September 22, 2022 10:06
@bee-worker bee-worker mentioned this pull request Sep 22, 2022
Merged
@bee-worker bee-worker mentioned this pull request Nov 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nugaon nugaon nugaon approved these changes

@vojtechsimetka vojtechsimetka Awaiting requested review from vojtechsimetka

@Cafe137 Cafe137 Awaiting requested review from Cafe137 Cafe137 is a code owner

Assignees
No one assigned
Labels
type:pull-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomicvladan @nugaon