Skip to content

Commit e169f81

Browse files
committed
Extract shared IsUriScheme helper; use std::ranges::replace (#341)
1 parent d30d697 commit e169f81

4 files changed

Lines changed: 69 additions & 33 deletions

File tree

src/iceberg/arrow/arrow_io.cc

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919

2020
#include <algorithm>
21-
#include <cctype>
2221
#include <chrono>
2322
#include <limits>
2423
#include <mutex>
@@ -37,6 +36,7 @@
3736
#include "iceberg/arrow/arrow_io_util.h"
3837
#include "iceberg/arrow/arrow_status_internal.h"
3938
#include "iceberg/util/macros.h"
39+
#include "iceberg/util/uri.h"
4040

4141
namespace iceberg::arrow {
4242

@@ -486,26 +486,15 @@ class ArrowOutputFile : public OutputFile {
486486

487487
Result<std::string> ArrowFileSystemFileIO::ResolvePath(const std::string& file_location) {
488488
// Detect whether the location is a URI by looking for a scheme component.
489-
// A URI scheme (RFC 3986 §3.1) starts with a letter and is followed by any
490-
// combination of letters, digits, '+', '-', or '.', ending at the first ':'.
491-
// A single character before ':' is a Windows drive letter (e.g., "C:\..."),
492-
// not a URI scheme.
493-
auto colon_pos = file_location.find(':');
494-
bool is_uri =
495-
colon_pos != std::string::npos && colon_pos > 1 &&
496-
std::isalpha(static_cast<unsigned char>(file_location[0])) &&
497-
std::all_of(file_location.begin(),
498-
file_location.begin() + static_cast<std::ptrdiff_t>(colon_pos),
499-
[](char c) {
500-
return std::isalpha(static_cast<unsigned char>(c)) ||
501-
std::isdigit(static_cast<unsigned char>(c)) || c == '+' ||
502-
c == '-' || c == '.';
503-
});
489+
// See iceberg/util/uri.h for the RFC 3986 scheme grammar.
490+
bool is_uri = IsUriScheme(file_location);
504491

505492
if (!is_uri) {
506493
return file_location; // Bare local path (Unix or Windows drive letter)
507494
}
508495

496+
auto colon_pos = file_location.find(':');
497+
509498
// Normalize authority-less file: URI short forms to the canonical three-slash
510499
// form so that Arrow's PathFromUri can parse them. Java Iceberg may write
511500
// "file:/path" (one slash) which lacks the "://" that Arrow expects.

src/iceberg/catalog/rest/rest_file_io.cc

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
#include "iceberg/catalog/rest/rest_file_io.h"
2121

22-
#include <algorithm>
23-
#include <cctype>
2422
#include <string>
2523
#include <unordered_map>
2624
#include <utility>
@@ -30,6 +28,7 @@
3028
#include "iceberg/file_io.h"
3129
#include "iceberg/file_io_registry.h"
3230
#include "iceberg/util/macros.h"
31+
#include "iceberg/util/uri.h"
3332

3433
namespace iceberg::rest {
3534

@@ -53,24 +52,15 @@ std::unordered_map<std::string, std::string> MergeFileIOProperties(
5352
} // namespace
5453

5554
Result<BuiltinFileIOKind> DetectBuiltinFileIO(std::string_view location) {
56-
// Detect URI scheme using RFC 3986 rules: a scheme is 2+ chars starting with
57-
// a letter, followed by letters/digits/+/-/., ending at ':'.
58-
// A single char before ':' is a Windows drive letter, not a scheme.
59-
auto colon_pos = location.find(':');
60-
bool is_uri =
61-
colon_pos != std::string_view::npos && colon_pos > 1 &&
62-
std::isalpha(static_cast<unsigned char>(location[0])) &&
63-
std::all_of(location.begin(),
64-
location.begin() + static_cast<std::ptrdiff_t>(colon_pos), [](char c) {
65-
return std::isalpha(static_cast<unsigned char>(c)) ||
66-
std::isdigit(static_cast<unsigned char>(c)) || c == '+' ||
67-
c == '-' || c == '.';
68-
});
55+
// Detect URI scheme using RFC 3986 rules.
56+
// See iceberg/util/uri.h for the scheme grammar.
57+
bool is_uri = IsUriScheme(location);
6958

7059
if (!is_uri) {
7160
return BuiltinFileIOKind::kArrowLocal;
7261
}
7362

63+
const auto colon_pos = location.find(':');
7464
const auto scheme = location.substr(0, colon_pos);
7565
if (scheme == "file") {
7666
return BuiltinFileIOKind::kArrowLocal;

src/iceberg/test/arrow_io_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ TEST_F(LocalFileIOTest, ResolvesFileUriSingleSlash) {
451451
// Build a cross-platform file:/path URI: normalize backslashes to forward slashes
452452
// and ensure a leading '/' (Linux paths already have one; Windows drive letters don't).
453453
std::string p = temp_filepath_;
454-
std::replace(p.begin(), p.end(), '\\', '/');
454+
std::ranges::replace(p, '\\', '/');
455455
std::string single_slash_uri = "file:" + (p.front() == '/' ? p : "/" + p);
456456
auto read_res = file_io_->ReadFile(single_slash_uri, std::nullopt);
457457
EXPECT_THAT(read_res, IsOk());
@@ -465,7 +465,7 @@ TEST_F(LocalFileIOTest, ResolvesFileUriTripleSlash) {
465465
// that "file://" + "/" + path produces the canonical triple-slash form on all
466466
// platforms. Linux: file:///tmp/... Windows: file:///C:/Users/...
467467
std::string p = temp_filepath_;
468-
std::replace(p.begin(), p.end(), '\\', '/');
468+
std::ranges::replace(p, '\\', '/');
469469
std::string triple_slash_uri = "file://" + (p.front() == '/' ? p : "/" + p);
470470
auto read_res = file_io_->ReadFile(triple_slash_uri, std::nullopt);
471471
EXPECT_THAT(read_res, IsOk());

src/iceberg/util/uri.h

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
#include <algorithm>
23+
#include <cctype>
24+
#include <string_view>
25+
26+
/// \file iceberg/util/uri.h
27+
/// \brief URI scheme detection utilities per RFC 3986.
28+
29+
namespace iceberg {
30+
31+
/// \brief Check whether a string begins with a valid RFC 3986 URI scheme
32+
/// followed by ':'.
33+
///
34+
/// A scheme (RFC 3986 §3.1) is defined as:
35+
/// scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
36+
///
37+
/// A single character before ':' is treated as a Windows drive letter, not a
38+
/// scheme (e.g., "C:\path").
39+
///
40+
/// \param value The string to inspect.
41+
/// \return true if \p value starts with a valid URI scheme followed by ':'.
42+
inline bool IsUriScheme(std::string_view value) {
43+
auto colon_pos = value.find(':');
44+
if (colon_pos == std::string_view::npos || colon_pos <= 1) {
45+
return false;
46+
}
47+
if (!std::isalpha(static_cast<unsigned char>(value[0]))) {
48+
return false;
49+
}
50+
return std::ranges::all_of(value.substr(1, colon_pos - 1), [](char c) {
51+
return std::isalpha(static_cast<unsigned char>(c)) ||
52+
std::isdigit(static_cast<unsigned char>(c)) || c == '+' || c == '-' ||
53+
c == '.';
54+
});
55+
}
56+
57+
} // namespace iceberg

0 commit comments

Comments
 (0)