From f9799cb312906782eea678b5a36468f0b0ed6a9f Mon Sep 17 00:00:00 2001
From: Henrique Dias <hacdias@gmail.com>
Date: Wed, 27 Sep 2023 10:44:59 +0200
Subject: [PATCH] fix: path joinining

---
 namesys/dns_resolver.go  |  7 +++----
 namesys/ipns_resolver.go |  9 ++++-----
 namesys/namesys.go       | 17 +++++++----------
 namesys/utilities.go     | 19 +++++++++++++++++++
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/namesys/dns_resolver.go b/namesys/dns_resolver.go
index 55c86d8d0..7622b7644 100644
--- a/namesys/dns_resolver.go
+++ b/namesys/dns_resolver.go
@@ -56,8 +56,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, options
 		return out
 	}
 
-	segments := p.Segments()
-	fqdn := segments[1]
+	fqdn := p.Segments()[1]
 	if _, ok := dns.IsDomainName(fqdn); !ok {
 		out <- ResolveResult{Err: fmt.Errorf("not a valid domain name: %q", fqdn)}
 		close(out)
@@ -90,7 +89,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, options
 					break
 				}
 				if subRes.Err == nil {
-					p, err := path.Join(subRes.Path, segments[2:]...)
+					p, err := joinPaths(subRes.Path, p)
 					emitOnceResult(ctx, out, ResolveResult{Path: p, Err: err})
 					// Return without waiting for rootRes, since this result
 					// (for "_dnslink."+fqdn) takes precedence
@@ -103,7 +102,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, options
 					break
 				}
 				if rootRes.Err == nil {
-					p, err := path.Join(rootRes.Path, segments[2:]...)
+					p, err := joinPaths(rootRes.Path, p)
 					emitOnceResult(ctx, out, ResolveResult{Path: p, Err: err})
 					// Do not return here.  Wait for subRes so that it is
 					// output last if good, thereby giving subRes precedence.
diff --git a/namesys/ipns_resolver.go b/namesys/ipns_resolver.go
index 83a32afb8..d46339f5d 100644
--- a/namesys/ipns_resolver.go
+++ b/namesys/ipns_resolver.go
@@ -68,8 +68,7 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, option
 		ctx, cancel = context.WithTimeout(ctx, options.DhtTimeout)
 	}
 
-	segments := p.Segments()
-	name, err := ipns.NameFromString(segments[1])
+	name, err := ipns.NameFromString(p.Segments()[1])
 	if err != nil {
 		out <- ResolveResult{Err: err}
 		close(out)
@@ -104,13 +103,13 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, option
 					return
 				}
 
-				p, err := rec.Value()
+				resolvedBase, err := rec.Value()
 				if err != nil {
 					emitOnceResult(ctx, out, ResolveResult{Err: err})
 					return
 				}
 
-				p, err = path.Join(p, segments[2:]...)
+				resolvedBase, err = joinPaths(resolvedBase, p)
 				if err != nil {
 					emitOnceResult(ctx, out, ResolveResult{Err: err})
 					return
@@ -122,7 +121,7 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, p path.Path, option
 					return
 				}
 
-				emitOnceResult(ctx, out, ResolveResult{Path: p, TTL: ttl})
+				emitOnceResult(ctx, out, ResolveResult{Path: resolvedBase, TTL: ttl})
 			case <-ctx.Done():
 				return
 			}
diff --git a/namesys/namesys.go b/namesys/namesys.go
index 00282ac6b..6753ff47f 100644
--- a/namesys/namesys.go
+++ b/namesys/namesys.go
@@ -31,6 +31,7 @@ import (
 	madns "github.com/multiformats/go-multiaddr-dns"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
+	"go.uber.org/multierr"
 )
 
 // namesys is a multi-protocol [NameSystem] that implements generic IPFS naming.
@@ -162,8 +163,6 @@ func (ns *namesys) resolveOnceAsync(ctx context.Context, p path.Path, options Re
 		return out
 	}
 
-	// TODO
-
 	segments := p.Segments()
 	resolvablePath, err := path.NewPathFromSegments(segments[0], segments[1])
 	if err != nil {
@@ -172,10 +171,8 @@ func (ns *namesys) resolveOnceAsync(ctx context.Context, p path.Path, options Re
 		return out
 	}
 
-	if p, ttl, ok := ns.cacheGet(resolvablePath.String()); ok {
-		if len(segments) > 2 {
-			p, err = path.Join(p, segments[2:]...)
-		}
+	if resolvedBase, ttl, ok := ns.cacheGet(resolvablePath.String()); ok {
+		p, err = joinPaths(resolvedBase, p)
 		span.SetAttributes(attribute.Bool("CacheHit", true))
 		span.RecordError(err)
 		out <- ResolveResult{Path: p, TTL: ttl, Err: err}
@@ -218,10 +215,10 @@ func (ns *namesys) resolveOnceAsync(ctx context.Context, p path.Path, options Re
 					best = res
 				}
 
-				// Attach rest of the path
-				p := res.Path
-				if p != nil && len(segments) > 2 {
-					p, err = path.Join(p, segments[2:]...)
+				p, err := joinPaths(res.Path, p)
+				if err != nil {
+					// res.Err may already be defined, so just combine them
+					res.Err = multierr.Combine(err, res.Err)
 				}
 
 				emitOnceResult(ctx, out, ResolveResult{Path: p, TTL: res.TTL, Err: res.Err})
diff --git a/namesys/utilities.go b/namesys/utilities.go
index 189302963..11854144e 100644
--- a/namesys/utilities.go
+++ b/namesys/utilities.go
@@ -3,6 +3,7 @@ package namesys
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/ipfs/boxo/path"
@@ -121,6 +122,24 @@ func emitResult(ctx context.Context, outCh chan<- ResolveResult, r ResolveResult
 	}
 }
 
+func joinPaths(resolvedBase, unresolvedPath path.Path) (path.Path, error) {
+	if resolvedBase == nil {
+		return nil, nil
+	}
+
+	segments := unresolvedPath.Segments()[2:]
+	if strings.HasSuffix(unresolvedPath.String(), "/") {
+		segments = append(segments, "")
+	}
+
+	// simple optimization
+	if len(segments) == 0 {
+		return resolvedBase, nil
+	}
+
+	return path.Join(resolvedBase, segments...)
+}
+
 var tracer = otel.Tracer("boxo/namesys")
 
 func startSpan(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {