Compare commits

..

11 Commits

Author SHA1 Message Date
AK-Khan02
16633e3c65 Return 400 for missing /api/serialize IDs (#1272) 2026-06-10 18:48:33 -07:00
Floyd Horng
d12120bab6 Fix syncback not removing stale $properties at engine defaults (#1244) 2026-06-10 18:47:50 -07:00
boatbomber
ac6941f054 Add origin/host validation and warning for exposed serves (#1270) 2026-06-07 15:51:05 -07:00
Ken Loeffler
444dc11b26 Fix init.plugin.lua/init.plugin.luau not being supported (#1252) 2026-06-07 15:50:04 -07:00
EgoMoose
15939b1647 Add ignorable glob support (#1256) 2026-06-02 20:40:12 -07:00
AK-Khan02
0afb26e143 Update stale README and contributor documentation (#1269) 2026-06-02 09:11:41 -07:00
boatbomber
1abf675949 Graceful errors instead of crashing (#1267) 2026-06-01 20:02:39 -07:00
boatbomber
85655ca84f Heuristic hydration matching instead of hard dependency on GetChildren ordering (#1266)
### Summary

When two or more sibling instances share the same `Name` and
`ClassName`, Rojo's reconciler previously paired them with their
server-side counterparts purely by child order (first-unvisited match in
`GetChildren()` order). If the workspace child order ever diverged from
the server's, the wrong instance got paired so each duplicate could
inherit a sibling's properties.

This is the root of the #1257 bug: the welded parts would oscillate
between positions on each connect/disconnect because hydration kept
mis-pairing them. (#1265 stopped the sync fallback from scrambling child
order in the first place but this PR makes hydration robust even when
order *does* diverge.)

This PR makes `hydrate` break ties by comparing properties: when several
existing children match on `Name`+`ClassName`, it scores each candidate
by how many of the virtual instance's properties match the candidate's
live values, and picks the best. Order remains the tiebreak when scores
are equal, so behavior is unchanged for uniquely-named instances and for
indistinguishable siblings.

### Changes

- `trueEquals.lua`: extracted verbatim from `diff.lua` (the fuzzy
value-equality helper) so it can be shared. No behavior change;
`diff.lua` now requires it.
- `countMatchingProperties.lua`: added
`countMatchingProperties(instance, virtualInstance, instanceMap) ->
number`. Skips `Ref` properties (the instanceMap isn't fully built
mid-hydrate, so refs can't be decoded reliably, and they're a poor
disambiguator anyway) and any property that can't be read or decoded.
- `hydrate.lua`: See details below.

### Hydrate Changes

This touches `hydrate`, which runs over the whole tree on every
connect/resync, so I want state clearly that **the common path is faster
than before, not slower** even for parents with thousands of children!

The old algorithm was a nested scan: for each of `V` virtual children,
scan existing children until the first unvisited `Name`+`ClassName`
match. Two costs stand out:
- A `pcall` (to guard DataModel permission errors) ran on every
comparison (up to `V*E` `pcall`s per parent).
- Even for in-order trees the re-scanning of the visited prefix made it
`O(V^2)`.

The new algorithm does a single bucketing pass, then `O(1)` lookups:
1. One `O(E)` pass groups existing children into nested
`buckets[name][className]` tables. This runs exactly `E` `pcall`s total
(one per child), down from the `V*E` worst case.
2. Each virtual child does an `O(1)` bucket lookup to find its
candidates.
3. A per-bucket cursor skips already-paired children, so order-based
matching is amortized `O(1)` per child instead of rescanning.

| Scenario | Old | New |
| ------------------------------------------------ |
--------------------------------------- |
--------------------------------------------- |
| Unique-named children (typical, incl. thousands) | `O(V^2)`, plus up
to `V*E` pcalls | `O(V + E)`, plus exactly `E` pcalls |
| `C <= 32` candidates | `O(C^2)` | `O(P * C^2)` scoring |
| `C > 32` candidates | `O(C^2)` | `O(C)` |

Property scoring (`getProperty`/`decodeValue`/`trueEquals`) is the only
new expense, and it's gated two ways:
- It runs only when a `Name`+`ClassName` group has >=2 candidates (i.e.
never for uniquely-named instances).
- A cap, `MAX_CANDIDATES_TO_SCORE = 32`, means scoring only kicks in
once a group has <=32 unvisited candidates. A folder of thousands of
identically-named parts therefore does not trigger scoring; it falls
back to the original order-based pairing. The worst-case added scoring
work is bounded to roughly `32^2` property comparisons per group,
independent of group size.

So overall this is faster when you have unique names or many children.
It is slower but more robust when you have small groups of duplicate
names. Memory usage is increased as it creates the candidate buckets.
2026-06-01 17:29:22 -07:00
boatbomber
ae8735c80a Fix replaceInstances messing up GetChildren ordering (#1265)
Fixes #1257. replaceInstances now preserves GetChildren ordering.

I also had the unit tests unreliably say that Packages wasn't a member
of ReplicatedStorage so I added WaitForChild and tests run 100% for me
now.
2026-06-01 17:04:25 -07:00
Ken Loeffler
988efb45b1 Use lossy conversion for msgpack UInt64 decode in plugin (#1255) 2026-05-29 10:45:08 -07:00
Micah
9bbb1edd79 Upload plugin as part of release workflow (#1227) 2026-02-14 11:10:36 -08:00
105 changed files with 2444 additions and 1641 deletions

View File

@@ -44,6 +44,13 @@ jobs:
with:
name: Rojo.rbxm
path: Rojo.rbxm
- name: Upload Plugin to Roblox
env:
RBX_API_KEY: ${{ secrets.PLUGIN_UPLOAD_TOKEN }}
RBX_UNIVERSE_ID: ${{ vars.PLUGIN_CI_PLACE_ID }}
RBX_PLACE_ID: ${{ vars.PLUGIN_CI_UNIVERSE_ID }}
run: lune run upload-plugin Rojo.rbxm
build:
needs: ["create-release"]

3
.gitmodules vendored
View File

@@ -19,3 +19,6 @@
[submodule "plugin/Packages/msgpack-luau"]
path = plugin/Packages/msgpack-luau
url = https://github.com/cipharius/msgpack-luau/
[submodule ".lune/opencloud-execute"]
path = .lune/opencloud-execute
url = https://github.com/Dekkonot/opencloud-luau-execute-lune.git

8
.lune/.config.luau Normal file
View File

@@ -0,0 +1,8 @@
return {
luau = {
languagemode = "strict",
aliases = {
lune = "~/.lune/.typedefs/0.10.4/",
},
},
}

View File

@@ -0,0 +1,51 @@
local args: any = ...
assert(args, "no arguments passed to script")
local input: buffer = args.BinaryInput
local AssetService = game:GetService("AssetService")
local SerializationService = game:GetService("SerializationService")
local EncodingService = game:GetService("EncodingService")
local input_hash: buffer = EncodingService:ComputeBufferHash(input, Enum.HashAlgorithm.Sha256)
local hex_hash: { string } = table.create(buffer.len(input_hash))
for i = 0, buffer.len(input_hash) - 1 do
table.insert(hex_hash, string.format("%02x", buffer.readu8(input_hash, i)))
end
print(`Deserializing plugin file (size: {buffer.len(input)} bytes, hash: {table.concat(hex_hash, "")})`)
local plugin = SerializationService:DeserializeInstancesAsync(input)[1]
local UploadDetails = require(plugin.UploadDetails) :: any
local PLUGIN_ID = UploadDetails.assetId
local PLUGIN_NAME = UploadDetails.name
local PLUGIN_DESCRIPTION = UploadDetails.description
local PLUGIN_CREATOR_ID = UploadDetails.creatorId
local PLUGIN_CREATOR_TYPE = UploadDetails.creatorType
assert(typeof(PLUGIN_ID) == "number", "UploadDetails did not contain a number field 'assetId'")
assert(typeof(PLUGIN_NAME) == "string", "UploadDetails did not contain a string field 'name'")
assert(typeof(PLUGIN_DESCRIPTION) == "string", "UploadDetails did not contain a string field 'description'")
assert(typeof(PLUGIN_CREATOR_ID) == "number", "UploadDetails did not contain a number field 'creatorId'")
assert(typeof(PLUGIN_CREATOR_TYPE) == "string", "UploadDetails did not contain a string field 'creatorType'")
assert(
Enum.AssetCreatorType:FromName(PLUGIN_CREATOR_TYPE) ~= nil,
"UploadDetails field 'creatorType' was not a valid member of Enum.AssetCreatorType"
)
print(`Uploading to {PLUGIN_ID}`)
print(`Plugin Name: {PLUGIN_NAME}`)
print(`Plugin Description: {PLUGIN_DESCRIPTION}`)
local result, version_or_err = AssetService:CreateAssetVersionAsync(plugin, Enum.AssetType.Plugin, PLUGIN_ID, {
["Name"] = PLUGIN_NAME,
["Description"] = PLUGIN_DESCRIPTION,
["CreatorId"] = PLUGIN_CREATOR_ID,
["CreatorType"] = Enum.AssetCreatorType:FromName(PLUGIN_CREATOR_TYPE),
})
if result ~= Enum.CreateAssetResult.Success then
error(`Plugin failed to upload because: {result.Name} - {version_or_err}`)
end
print(`Plugin uploaded successfully. New version is {version_or_err}.`)

78
.lune/upload-plugin.luau Normal file
View File

@@ -0,0 +1,78 @@
local fs = require("@lune/fs")
local process = require("@lune/process")
local stdio = require("@lune/stdio")
local luau_execute = require("./opencloud-execute")
local UNIVERSE_ID = process.env["RBX_UNIVERSE_ID"]
local PLACE_ID = process.env["RBX_PLACE_ID"]
local version_string = fs.readFile("plugin/Version.txt")
local versions = { string.match(version_string, "^v?(%d+)%.(%d+)%.(%d+)(.*)$") }
if versions[4] ~= "" then
print("This release is a pre-release. Skipping uploading plugin.")
process.exit(0)
end
local plugin_path = process.args[1]
assert(
typeof(plugin_path) == "string",
"no plugin path provided, expected usage is `lune run upload-plugin [PATH TO RBXM]`."
)
-- For local testing
if process.env["CI"] ~= "true" then
local rojo = process.exec("rojo", { "build", "plugin.project.json", "--output", plugin_path })
if not rojo.ok then
stdio.ewrite("plugin upload failed because: could not build plugin.rbxm\n\n")
stdio.ewrite(rojo.stderr)
stdio.ewrite("\n")
process.exit(1)
end
else
assert(fs.isFile(plugin_path), `Plugin file did not exist at {plugin_path}`)
end
local plugin_content = fs.readFile(plugin_path)
local engine_script = fs.readFile(".lune/scripts/plugin-upload.luau")
print("Creating task to upload plugin")
local task = luau_execute.create_task_latest(UNIVERSE_ID, PLACE_ID, engine_script, 300, false, plugin_content)
print("Waiting for task to finish")
local success = luau_execute.await_finish(task)
if not success then
local error = luau_execute.get_error(task)
assert(error, "could not fetch error from task")
stdio.ewrite("plugin upload failed because: task did not finish successfully\n\n")
stdio.ewrite(error.code)
stdio.ewrite("\n")
stdio.ewrite(error.message)
stdio.ewrite("\n")
process.exit(1)
end
print("Output from task:\n")
for _, log in luau_execute.get_structured_logs(task) do
if log.messageType == "ERROR" then
stdio.write(stdio.color("red"))
stdio.write(log.message)
stdio.write("\n")
stdio.write(stdio.color("reset"))
elseif log.messageType == "INFO" then
stdio.write(stdio.color("cyan"))
stdio.write(log.message)
stdio.write("\n")
stdio.write(stdio.color("reset"))
elseif log.messageType == "WARNING" then
stdio.write(stdio.color("yellow"))
stdio.write(log.message)
stdio.write("\n")
stdio.write(stdio.color("reset"))
else
stdio.write(stdio.color("reset"))
stdio.write(log.message)
stdio.write("\n")
stdio.write(stdio.color("reset"))
end
end

View File

@@ -33,23 +33,37 @@ Making a new release? Simply add the new header with the version and date undern
* `inf` and `nan` values in properties are now synced ([#1176])
* Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179])
* Implemented support for the "name" property in meta/model JSON files. ([#1187])
* Fixed instance replacement fallback failing when too many instances needed to be replaced. ([#1192])
* Added actors and bindable/remote event/function variants to be synced back as JSON files. ([#1199])
* Fixed a bug where MacOS paths weren't being handled correctly. ([#1201])
* Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211])
* Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215])
* Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217])
* Fixed missing support for init.plugin.lua and init.plugin.luau. ([#1252])
* Add support for gitignore-style negation in `globIgnorePaths` and syncback's `ignorePaths` ([#1256])
* Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265])
* Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266])
* Rojo now reports a clear error instead of panicking in several cases, including when the `serve` port is already in use, when a synced file is read-only or locked, when the filesystem watcher can't be created, and when the working directory is inaccessible. ([#1267])
* Fixed `/api/serialize` returning success when a requested instance ID is missing from the serve session tree. ([#1272])
* `rojo serve` now validates the `Host`/`Origin` headers to protect the local/private server against DNS rebinding, gates `/api/open` to local clients, and warns when bound to a network-reachable address. The accepted hosts can be extended with the `--allowed-hosts` option or a project's `serveAllowedHosts` field, for example to reach a network-exposed server by hostname. ([#1270])
* Fixed syncback not removing stale `$properties` entries when Studio resets a property to its engine default. ([#1244])
[#1176]: https://github.com/rojo-rbx/rojo/pull/1176
[#1179]: https://github.com/rojo-rbx/rojo/pull/1179
[#1187]: https://github.com/rojo-rbx/rojo/pull/1187
[#1192]: https://github.com/rojo-rbx/rojo/pull/1192
[#1199]: https://github.com/rojo-rbx/rojo/pull/1199
[#1201]: https://github.com/rojo-rbx/rojo/pull/1201
[#1211]: https://github.com/rojo-rbx/rojo/pull/1211
[#1215]: https://github.com/rojo-rbx/rojo/pull/1215
[#1217]: https://github.com/rojo-rbx/rojo/pull/1217
[#1252]: https://github.com/rojo-rbx/rojo/pull/1252
[#1256]: https://github.com/rojo-rbx/rojo/pull/1256
[#1265]: https://github.com/rojo-rbx/rojo/pull/1265
[#1266]: https://github.com/rojo-rbx/rojo/pull/1266
[#1267]: https://github.com/rojo-rbx/rojo/pull/1267
[#1272]: https://github.com/rojo-rbx/rojo/pull/1272
[#1270]: https://github.com/rojo-rbx/rojo/pull/1270
[#1244]: https://github.com/rojo-rbx/rojo/pull/1244
## [7.7.0-rc.1] (November 27th, 2025)

View File

@@ -13,12 +13,27 @@ Code contributions are welcome for features and bugs that have been reported in
You'll want these tools to work on Rojo:
* Latest stable Rust compiler
* Rust 1.88 or newer
* Rustfmt and Clippy are used for code formatting and linting.
* Latest stable [Rojo](https://github.com/rojo-rbx/rojo)
* [Rokit](https://github.com/rojo-rbx/rokit)
* [Luau Language Server](https://github.com/JohnnyMorganz/luau-lsp) (Only needed if working on the Studio plugin.)
Rokit installs the pinned Rojo, Selene, StyLua, Lune, and run-in-roblox versions listed in [`rokit.toml`](rokit.toml):
```bash
rokit install
```
Before opening a pull request, run the relevant checks:
```bash
cargo test
cargo fmt -- --check
cargo clippy
stylua --check plugin/src
selene plugin/src
```
When working on the Studio plugin, we recommend using this command to automatically rebuild the plugin when you save a change:
*(Make sure you've enabled the Studio setting to reload plugins on file change!)*
@@ -29,7 +44,7 @@ bash scripts/watch-build-plugin.sh
You can also run the plugin's unit tests with the following:
*(Make sure you have `run-in-roblox` installed first!)*
*(If you are not using Rokit, make sure you have `run-in-roblox` installed first!)*
```bash
bash scripts/unit-test-plugin.sh
@@ -49,27 +64,27 @@ Please file issues and we'll try to help figure out what the best way forward is
## Local Development Gotchas
If your build fails with "Error: failed to open file `D:\code\rojo\plugin\modules\roact\src`" you need to update your Git submodules.
If your build fails with an error about a missing path under `plugin/Packages`, such as `plugin/Packages/Roact`, you need to update your Git submodules.
Run the command and try building again: `git submodule update --init --recursive`.
## Pushing a Rojo Release
The Rojo release process is pretty manual right now. If you need to do it, here's how:
The Rojo release process is driven by the GitHub Actions release workflow. If you need to do it, here's how:
1. Bump server version in [`Cargo.toml`](Cargo.toml)
2. Bump plugin version in [`plugin/src/Config.lua`](plugin/src/Config.lua)
3. Run `cargo test` to update `Cargo.lock` and run tests
2. Bump plugin version in [`plugin/Version.txt`](plugin/Version.txt)
* The build checks that the Cargo and plugin versions match.
3. Run `cargo test` to update `Cargo.lock` after the version bump and run tests
4. Update [`CHANGELOG.md`](CHANGELOG.md)
5. Commit!
* `git add . && git commit -m "Release vX.Y.Z"`
6. Tag the commit
* `git tag vX.Y.Z`
7. Publish the CLI
* `cargo publish`
8. Publish the Plugin
* `cargo run -- upload plugin --asset_id 6415005344`
9. Push commits and tags
7. Push commits and tags
* `git push && git push --tags`
8. Wait for the GitHub Actions release workflow to create the draft release and upload CLI/plugin artifacts
9. Publish the CLI crate
* `cargo publish`
10. Copy GitHub release content from previous release
* Update the leading text with a summary about the release
* Paste the changelog notes (as-is!) from [`CHANGELOG.md`](CHANGELOG.md)
* Write a small summary of each major feature
* Write a small summary of each major feature

View File

@@ -25,12 +25,11 @@ Rojo enables:
* Versioning your game, library, or plugin using Git or another VCS
* Streaming `rbxmx` and `rbxm` models into your game in real time
* Packaging and deploying your project to Roblox.com from the command line
* Pulling Instances from Roblox place and model files back into an existing Rojo project with `rojo syncback`
In the future, Rojo will be able to:
Rojo also has an optional two-way sync setting in the Studio plugin for syncing supported Studio edits back to the filesystem.
* Sync instances from Roblox Studio to the filesystem
* Automatically convert your existing game to work with Rojo
* Import custom instances like MoonScript code
Some workflows, like fully automatic conversion of every existing game into a Rojo project, are still limited and may require manual project configuration.
## [Documentation](https://rojo.space/docs)
Documentation is hosted in the [rojo.space repository](https://github.com/rojo-rbx/rojo.space).

View File

@@ -75,6 +75,7 @@ fn main() -> Result<(), anyhow::Error> {
"src" => snapshot_from_fs_path(&plugin_dir.join("src"))?,
"Packages" => snapshot_from_fs_path(&plugin_dir.join("Packages"))?,
"Version.txt" => snapshot_from_fs_path(&plugin_dir.join("Version.txt"))?,
"UploadDetails.json" => snapshot_from_fs_path(&plugin_dir.join("UploadDetails.json"))?,
}),
});

View File

@@ -2,6 +2,9 @@
## Unreleased Changes
* Added `Vfs::canonicalize`. [#1201]
* **Breaking:** `StdBackend::new` and `Vfs::new_default` now return `io::Result`, so a failure to create the filesystem watcher is reported as an error instead of panicking. The `Default` implementation for `StdBackend` has been removed as a result. [#1267]
[#1267]: https://github.com/rojo-rbx/rojo/pull/1267
## 0.3.1 (2025-11-27)
* Added `Vfs::exists`. [#1169]

View File

@@ -255,8 +255,11 @@ pub struct Vfs {
impl Vfs {
/// Creates a new `Vfs` with the default backend, `StdBackend`.
pub fn new_default() -> Self {
Self::new(StdBackend::new())
///
/// Returns an error if the filesystem watcher could not be initialized,
/// which can happen in restricted or sandboxed environments.
pub fn new_default() -> io::Result<Self> {
Ok(Self::new(StdBackend::new()?))
}
/// Creates a new `Vfs` with the given backend.
@@ -639,7 +642,7 @@ mod test {
let file_path = dir.path().join("file.txt");
fs_err::write(&file_path, contents.to_string()).unwrap();
let vfs = Vfs::new(StdBackend::new());
let vfs = Vfs::new(StdBackend::new().unwrap());
let canonicalized = vfs.canonicalize(&file_path).unwrap();
assert_eq!(canonicalized, file_path.canonicalize().unwrap());
assert_eq!(
@@ -653,7 +656,7 @@ mod test {
let dir = tempfile::tempdir().unwrap();
let file_path = dir.path().join("test");
let vfs = Vfs::new(StdBackend::new());
let vfs = Vfs::new(StdBackend::new().unwrap());
let err = vfs.canonicalize(&file_path).unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::NotFound);
}

View File

@@ -17,9 +17,9 @@ pub struct StdBackend {
}
impl StdBackend {
pub fn new() -> StdBackend {
pub fn new() -> io::Result<StdBackend> {
let (notify_tx, notify_rx) = mpsc::channel();
let watcher = watcher(notify_tx, Duration::from_millis(50)).unwrap();
let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(io::Error::other)?;
let (tx, rx) = crossbeam_channel::unbounded();
@@ -46,11 +46,11 @@ impl StdBackend {
Result::<(), crossbeam_channel::SendError<VfsEvent>>::Ok(())
});
Self {
Ok(Self {
watcher,
watcher_receiver: rx,
watches: HashSet::new(),
}
})
}
}
@@ -134,9 +134,3 @@ impl VfsBackend for StdBackend {
self.watcher.unwatch(path).map_err(io::Error::other)
}
}
impl Default for StdBackend {
fn default() -> Self {
Self::new()
}
}

View File

@@ -22,6 +22,9 @@
},
"Version": {
"$path": "plugin/Version.txt"
},
"UploadDetails": {
"$path": "plugin/UploadDetails.json"
}
}
}

View File

@@ -0,0 +1,7 @@
{
"assetId": 13916111004,
"name": "Rojo",
"description": "The plugin portion of Rojo, a tool to enable professional tooling for Roblox developers.",
"creatorId": 32644114,
"creatorType": "Group"
}

View File

@@ -14,6 +14,13 @@ local Http = {}
Http.Error = HttpError
Http.Response = HttpResponse
-- Monkey patch msgpack.UInt64.new to lossily convert the low and high bits of the integer
-- to a native Luau number. We should change the upstream decoder to emit a native
-- integer, once those are live.
function msgpack.UInt64.new(mostSignificantPart: number, leastSignificantPart: number): number
return (mostSignificantPart % 2 ^ 32) * 2 ^ 32 + (leastSignificantPart % 2 ^ 32)
end
local function performRequest(requestParams)
local requestId = lastRequestId + 1
lastRequestId = requestId

View File

@@ -1,8 +1,8 @@
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TestEZ = require(ReplicatedStorage.Packages:WaitForChild("TestEZ", 10))
local TestEZ = require(ReplicatedStorage:WaitForChild("Packages", 10):WaitForChild("TestEZ", 10))
local Rojo = ReplicatedStorage.Rojo
local Rojo = ReplicatedStorage:WaitForChild("Rojo", 10)
local Settings = require(Rojo.Plugin.Settings)
Settings:set("logLevel", "Trace")

View File

@@ -0,0 +1,44 @@
--[[
Counts how many of a virtual instance's properties match the live values on a
candidate Roblox instance. `hydrate` uses this to break ties when several
existing children share the same Name and ClassName.
This mirrors the read -> decode -> compare flow that `diff` uses, reusing the
same `getProperty`, `decodeValue`, and `trueEquals` helpers.
]]
local getProperty = require(script.Parent.getProperty)
local decodeValue = require(script.Parent.decodeValue)
local trueEquals = require(script.Parent.trueEquals)
local function countMatchingProperties(instance, virtualInstance, instanceMap)
local score = 0
for propertyName, virtualValue in virtualInstance.Properties do
-- Skip refs. During hydration the instanceMap is still being built
-- top-down, so a ref may point at an instance we haven't hydrated yet
-- and therefore can't decode reliably. Refs are also a poor
-- disambiguator between same-named siblings.
if next(virtualValue) == "Ref" then
continue
end
local getSuccess, existingValue = getProperty(instance, propertyName)
if not getSuccess then
continue
end
local decodeSuccess, decodedValue = decodeValue(virtualValue, instanceMap)
if not decodeSuccess then
continue
end
if trueEquals(existingValue, decodedValue) then
score += 1
end
end
return score
end
return countMatchingProperties

View File

@@ -0,0 +1,91 @@
return function()
local countMatchingProperties = require(script.Parent.countMatchingProperties)
local InstanceMap = require(script.Parent.Parent.InstanceMap)
it("counts properties whose values match the instance", function()
local instance = Instance.new("StringValue")
instance.Value = "hello"
local virtualInstance = {
ClassName = "StringValue",
Name = "Value",
Properties = {
Value = { String = "hello" },
},
Children = {},
}
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
end)
it("does not count properties whose values differ", function()
local instance = Instance.new("StringValue")
instance.Value = "hello"
local virtualInstance = {
ClassName = "StringValue",
Name = "Value",
Properties = {
Value = { String = "different" },
},
Children = {},
}
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
end)
it("counts multiple matching properties independently", function()
local instance = Instance.new("Part")
instance.Anchored = true
instance.CanCollide = false
local virtualInstance = {
ClassName = "Part",
Name = "Part",
Properties = {
Anchored = { Bool = true },
CanCollide = { Bool = false },
},
Children = {},
}
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(2)
-- Flip one so only a single property matches.
instance.CanCollide = true
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
end)
it("skips unknown properties without counting or erroring", function()
local instance = Instance.new("Folder")
local virtualInstance = {
ClassName = "Folder",
Name = "Folder",
Properties = {
FAKE_PROPERTY = { String = "nope" },
},
Children = {},
}
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
end)
it("skips Ref properties without counting or erroring", function()
local instance = Instance.new("ObjectValue")
local virtualInstance = {
ClassName = "ObjectValue",
Name = "ObjectValue",
Properties = {
-- A ref must be skipped rather than decoded: during hydration
-- the target may not be in the map yet.
Value = { Ref = "00000000000000000000000000000000" },
},
Children = {},
}
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
end)
end

View File

@@ -10,104 +10,12 @@ local invariant = require(script.Parent.Parent.invariant)
local getProperty = require(script.Parent.getProperty)
local Error = require(script.Parent.Error)
local decodeValue = require(script.Parent.decodeValue)
local trueEquals = require(script.Parent.trueEquals)
local function isEmpty(table)
return next(table) == nil
end
local function fuzzyEq(a: number, b: number, epsilon: number): boolean
return math.abs(a - b) < epsilon
end
local function trueEquals(a, b): boolean
-- Exit early for simple equality values
if a == b then
return true
end
-- Treat nil and { Ref = "000...0" } as equal
if
(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
then
return true
end
local typeA, typeB = typeof(a), typeof(b)
-- For tables, try recursive deep equality
if typeA == "table" and typeB == "table" then
local checkedKeys = {}
for key, value in a do
checkedKeys[key] = true
if not trueEquals(value, b[key]) then
return false
end
end
for key, value in b do
if checkedKeys[key] then
continue
end
if not trueEquals(value, a[key]) then
return false
end
end
return true
-- For NaN, check if both values are not equal to themselves
elseif a ~= a and b ~= b then
return true
-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "number" and typeB == "number" then
return fuzzyEq(a, b, 0.0001)
-- For EnumItem->number, compare the EnumItem's value
elseif typeA == "number" and typeB == "EnumItem" then
return a == b.Value
elseif typeA == "EnumItem" and typeB == "number" then
return a.Value == b
-- For Color3s, compare to RGB ints to avoid floating point inequality
elseif typeA == "Color3" and typeB == "Color3" then
local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
return aR == bR and aG == bG and aB == bB
-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "CFrame" and typeB == "CFrame" then
local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "Vector3" and typeB == "Vector3" then
local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "Vector2" and typeB == "Vector2" then
local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
end
return false
end
local function shouldDeleteUnknownInstances(virtualInstance)
if virtualInstance.Metadata ~= nil then
return not virtualInstance.Metadata.ignoreUnknownInstances

View File

@@ -3,9 +3,22 @@
concrete instances and assigning them IDs.
]]
local invariant = require(script.Parent.Parent.invariant)
local Packages = script.Parent.Parent.Parent.Packages
local Log = require(Packages.Log)
local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
local invariant = require(script.Parent.Parent.invariant)
local countMatchingProperties = require(script.Parent.countMatchingProperties)
-- When several existing children share a Name and ClassName we disambiguate
-- them by scoring how well each one's properties match the virtual instance.
-- That scoring is far more expensive than a Name/ClassName check, so we only do
-- it for reasonably-sized groups. Larger groups (e.g. a folder with thousands of
-- identically-named parts) fall back to the original order-based matching, which
-- bounds the added work to roughly MAX_CANDIDATES_TO_SCORE^2 property reads per
-- group regardless of how large the group is.
local MAX_CANDIDATES_TO_SCORE = 32
local function hydrateInner(stats, instanceMap, virtualInstances, rootId, rootInstance)
local virtualInstance = virtualInstances[rootId]
if virtualInstance == nil then
@@ -13,38 +26,163 @@ local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
end
instanceMap:insert(rootId, rootInstance)
stats.hydrated += 1
local existingChildren = rootInstance:GetChildren()
-- For each existing child, we'll track whether it's been paired with an
-- instance that the Rojo server knows about.
local isExistingChildVisited = {}
for i = 1, #existingChildren do
isExistingChildVisited[i] = false
-- Group existing children by Name then ClassName so each virtual child can
-- find its candidate matches without scanning every sibling. This is what
-- keeps hydration fast for parents with thousands of children. Nesting the
-- two tables (rather than a combined key) keeps the Name and ClassName checks
-- exact, with no way for one to bleed into the other.
local buckets = {}
for _, childInstance in existingChildren do
-- We guard accessing Name and ClassName in order to avoid tripping over
-- children of DataModel that Rojo won't have permissions to access at all.
local accessSuccess, name, className = pcall(function()
return childInstance.Name, childInstance.ClassName
end)
if not accessSuccess then
continue
end
local bucketsByClassName = buckets[name]
if bucketsByClassName == nil then
bucketsByClassName = {}
buckets[name] = bucketsByClassName
end
local bucket = bucketsByClassName[className]
if bucket == nil then
bucket = { cursor = 1, instances = {} }
bucketsByClassName[className] = bucket
end
table.insert(bucket.instances, childInstance)
end
-- Tracks which existing children have already been paired, so one instance
-- isn't matched to two different virtual instances.
local visited = {}
for _, childId in ipairs(virtualInstance.Children) do
local virtualChild = virtualInstances[childId]
for childIndex, childInstance in existingChildren do
if not isExistingChildVisited[childIndex] then
-- We guard accessing Name and ClassName in order to avoid
-- tripping over children of DataModel that Rojo won't have
-- permissions to access at all.
local accessSuccess, name, className = pcall(function()
return childInstance.Name, childInstance.ClassName
end)
local bucketsByClassName = buckets[virtualChild.Name]
local bucket = bucketsByClassName and bucketsByClassName[virtualChild.ClassName]
if bucket == nil then
-- No existing instance matches; diff will mark this id for creation.
Log.trace(
"hydrate: no existing instance matches {} ({}) for id {}",
virtualChild.Name,
virtualChild.ClassName,
childId
)
continue
end
-- This rule is very conservative and could be loosened in the
-- future, or more heuristics could be introduced.
if accessSuccess and name == virtualChild.Name and className == virtualChild.ClassName then
isExistingChildVisited[childIndex] = true
hydrate(instanceMap, virtualInstances, childId, childInstance)
break
local instances = bucket.instances
-- Advance past any leading children that have already been paired. The
-- cursor makes order-based matching amortized O(1) per child even for
-- very large groups, rather than rescanning the visited prefix.
while bucket.cursor <= #instances and visited[instances[bucket.cursor]] do
bucket.cursor += 1
end
if bucket.cursor > #instances then
-- Every matching instance has already been paired with an earlier id.
Log.trace(
"hydrate: no unpaired instance left for {} ({}) for id {}",
virtualChild.Name,
virtualChild.ClassName,
childId
)
continue
end
-- The cursor points at the earliest unvisited child, so the slots from
-- here to the end bound how many candidates remain. Visited children
-- after the cursor (gaps) only appear once a group is small enough to be
-- scored -- the order-based path below always takes the earliest, which
-- keeps the visited region a contiguous prefix. So whenever this count
-- exceeds the cap it is exact, and we can pick the earliest match without
-- collecting anything.
local remaining = #instances - bucket.cursor + 1
local match
if remaining > MAX_CANDIDATES_TO_SCORE then
-- Too many to score affordably; take the earliest in child order,
-- reproducing the original Name + ClassName behavior.
match = instances[bucket.cursor]
Log.trace(
"hydrate: {} candidates named {} ({}) exceeds the scoring cap of {}; matching id {} by child order",
remaining,
virtualChild.Name,
virtualChild.ClassName,
MAX_CANDIDATES_TO_SCORE,
childId
)
else
-- Collect the (at most `remaining`) unvisited candidates.
local candidates = {}
for index = bucket.cursor, #instances do
local childInstance = instances[index]
if not visited[childInstance] then
table.insert(candidates, childInstance)
end
end
if #candidates == 1 then
-- Only one candidate, so there's nothing to disambiguate.
match = candidates[1]
else
-- Break the tie by choosing the candidate whose properties best
-- match the virtual instance, falling back to the earliest in
-- child order when scores are equal.
local bestScore = -1
for _, childInstance in candidates do
local score = countMatchingProperties(childInstance, virtualChild, instanceMap)
if score > bestScore then
bestScore = score
match = childInstance
end
end
stats.ambiguousGroups += 1
stats.candidatesScored += #candidates
Log.trace(
"hydrate: disambiguated {} candidates named {} ({}) for id {} by property match (best score {})",
#candidates,
virtualChild.Name,
virtualChild.ClassName,
childId,
bestScore
)
end
end
visited[match] = true
hydrateInner(stats, instanceMap, virtualInstances, childId, match)
end
end
local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
-- Tallies of the work hydration did, surfaced in a single debug log below so
-- the cost of property-based disambiguation is visible without per-node spam.
local stats = {
hydrated = 0,
ambiguousGroups = 0,
candidatesScored = 0,
}
hydrateInner(stats, instanceMap, virtualInstances, rootId, rootInstance)
Log.debug(
"Hydrated {} instances ({} ambiguous name+class groups, {} candidates scored)",
stats.hydrated,
stats.ambiguousGroups,
stats.candidatesScored
)
end
return hydrate

View File

@@ -126,4 +126,140 @@ return function()
expect(knownInstances.fromIds["CHILD1"]).to.equal(child1)
expect(knownInstances.fromIds["CHILD2"]).to.equal(child2)
end)
it("should disambiguate duplicate-named siblings by matching properties", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = { "CHILD_A", "CHILD_B" },
},
CHILD_A = {
ClassName = "StringValue",
Name = "a",
Properties = { Value = { String = "first" } },
Children = {},
},
CHILD_B = {
ClassName = "StringValue",
Name = "a",
Properties = { Value = { String = "second" } },
Children = {},
},
}
local rootInstance = Instance.new("Folder")
-- Created in the reverse order of the virtual children, so a purely
-- order-based tiebreak would mis-pair them.
local child1 = Instance.new("StringValue")
child1.Name = "a"
child1.Value = "second"
child1.Parent = rootInstance
local child2 = Instance.new("StringValue")
child2.Name = "a"
child2.Value = "first"
child2.Parent = rootInstance
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(3)
expect(knownInstances.fromIds["CHILD_A"]).to.equal(child2)
expect(knownInstances.fromIds["CHILD_B"]).to.equal(child1)
end)
it("should fall back to child order for duplicate-named siblings with no distinguishing properties", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = { "CHILD_A", "CHILD_B" },
},
CHILD_A = {
ClassName = "Folder",
Name = "a",
Properties = {},
Children = {},
},
CHILD_B = {
ClassName = "Folder",
Name = "a",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
local child1 = Instance.new("Folder")
child1.Name = "a"
child1.Parent = rootInstance
local child2 = Instance.new("Folder")
child2.Name = "a"
child2.Parent = rootInstance
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(3)
-- With equal scores the earliest unvisited child wins, preserving the
-- original order-based behavior.
expect(knownInstances.fromIds["CHILD_A"]).to.equal(child1)
expect(knownInstances.fromIds["CHILD_B"]).to.equal(child2)
end)
it("should fall back to child order for very large duplicate-named groups", function()
-- More candidates than hydrate is willing to score at once. The group
-- must fall back to order-based matching, so virtual child N pairs with
-- existing child N regardless of properties.
local count = 64
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
local expectedInstances = {}
for i = 1, count do
local id = "CHILD_" .. i
table.insert(virtualInstances.ROOT.Children, id)
virtualInstances[id] = {
ClassName = "StringValue",
Name = "a",
-- Distinct values that, if scored, would pair by value rather
-- than by order.
Properties = { Value = { String = "value " .. i } },
Children = {},
}
local child = Instance.new("StringValue")
child.Name = "a"
child.Value = "value " .. (count - i + 1)
child.Parent = rootInstance
expectedInstances[id] = child
end
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(count + 1)
for id, expectedInstance in expectedInstances do
expect(knownInstances.fromIds[id]).to.equal(expectedInstance)
end
end)
end

View File

@@ -41,41 +41,14 @@ function reifyInstanceInner(unappliedPatch, deferredRefs, instanceMap, virtualIn
invariant("Cannot reify an instance not present in virtualInstances\nID: {}", id)
end
-- Before creating a new instance, check if the parent already has an
-- untracked child with the same Name and ClassName. This enables "late
-- adoption" of instances that exist in Studio but weren't in the initial
-- Rojo tree (e.g., when using --git-since filtering). Without this,
-- newly acknowledged files would create duplicate instances.
local adoptedExisting = false
local instance = nil
-- Instance.new can fail if we're passing in something that can't be
-- created, like a service, something enabled with a feature flag, or
-- something that requires higher security than we have.
local createSuccess, instance = pcall(Instance.new, virtualInstance.ClassName)
for _, child in ipairs(parentInstance:GetChildren()) do
local accessSuccess, name, className = pcall(function()
return child.Name, child.ClassName
end)
if accessSuccess
and name == virtualInstance.Name
and className == virtualInstance.ClassName
and instanceMap.fromInstances[child] == nil
then
instance = child
adoptedExisting = true
break
end
end
if not adoptedExisting then
-- Instance.new can fail if we're passing in something that can't be
-- created, like a service, something enabled with a feature flag, or
-- something that requires higher security than we have.
local createSuccess
createSuccess, instance = pcall(Instance.new, virtualInstance.ClassName)
if not createSuccess then
addAllToPatch(unappliedPatch, virtualInstances, id)
return
end
if not createSuccess then
addAllToPatch(unappliedPatch, virtualInstances, id)
return
end
-- TODO: Can this fail? Previous versions of Rojo guarded against this, but
@@ -123,9 +96,7 @@ function reifyInstanceInner(unappliedPatch, deferredRefs, instanceMap, virtualIn
reifyInstanceInner(unappliedPatch, deferredRefs, instanceMap, virtualInstances, childId, instance)
end
if not adoptedExisting then
instance.Parent = parentInstance
end
instance.Parent = parentInstance
instanceMap:insert(id, instance)
end

View File

@@ -0,0 +1,100 @@
--[[
Fuzzy value-equality used to compare a decoded virtual property value against
the live value read from a real instance. Shared by `diff` (to decide whether
a property changed) and `hydrate` (to score candidate instances).
]]
local function fuzzyEq(a: number, b: number, epsilon: number): boolean
return math.abs(a - b) < epsilon
end
local function trueEquals(a, b): boolean
-- Exit early for simple equality values
if a == b then
return true
end
-- Treat nil and { Ref = "000...0" } as equal
if
(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
then
return true
end
local typeA, typeB = typeof(a), typeof(b)
-- For tables, try recursive deep equality
if typeA == "table" and typeB == "table" then
local checkedKeys = {}
for key, value in a do
checkedKeys[key] = true
if not trueEquals(value, b[key]) then
return false
end
end
for key, value in b do
if checkedKeys[key] then
continue
end
if not trueEquals(value, a[key]) then
return false
end
end
return true
-- For NaN, check if both values are not equal to themselves
elseif a ~= a and b ~= b then
return true
-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "number" and typeB == "number" then
return fuzzyEq(a, b, 0.0001)
-- For EnumItem->number, compare the EnumItem's value
elseif typeA == "number" and typeB == "EnumItem" then
return a == b.Value
elseif typeA == "EnumItem" and typeB == "number" then
return a.Value == b
-- For Color3s, compare to RGB ints to avoid floating point inequality
elseif typeA == "Color3" and typeB == "Color3" then
local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
return aR == bR and aG == bG and aB == bB
-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "CFrame" and typeB == "CFrame" then
local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "Vector3" and typeB == "Vector3" then
local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
elseif typeA == "Vector2" and typeB == "Vector2" then
local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
for i, aComponent in aComponents do
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
return false
end
end
return true
end
return false
end
return trueEquals

View File

@@ -18,6 +18,7 @@ local PatchSet = require(script.Parent.PatchSet)
local Reconciler = require(script.Parent.Reconciler)
local strict = require(script.Parent.strict)
local Settings = require(script.Parent.Settings)
local orderSwaps = require(script.Parent.orderSwaps)
local Status = strict("Session.Status", {
NotStarted = "NotStarted",
@@ -320,6 +321,14 @@ function ServeSession:__replaceInstances(idList)
return false
end
-- Roblox appends to GetChildren() on every reparent, so the order in which
-- we re-parent replacements determines their final sibling order.
-- We process ancestors before descendants (so each replacement's
-- parent already exists when we re-parent it) and siblings in their original
-- GetChildren() order. Because the loop below moves the old instance's
-- children into the replacement *before* re-parenting the replacement, this
-- rebuilds GetChildren() exactly as it was before the swap.
local swaps = {}
for id, replacement in replacements do
local oldInstance = self.__instanceMap.fromIds[id]
if not oldInstance then
@@ -328,6 +337,16 @@ function ServeSession:__replaceInstances(idList)
continue
end
table.insert(swaps, {
id = id,
replacement = replacement,
oldInstance = oldInstance,
})
end
for _, swap in orderSwaps(swaps) do
local id, replacement, oldInstance = swap.id, swap.replacement, swap.oldInstance
self.__instanceMap:insert(id, replacement)
Log.trace("Swapping Instance {} out via api/models/ endpoint", id)
local oldParent = oldInstance.Parent

44
plugin/src/orderSwaps.lua Normal file
View File

@@ -0,0 +1,44 @@
--[[
Determines the order in which `ServeSession:__replaceInstances` should swap
instances so that sibling order is preserved.
Roblox appends to `GetChildren()` on every reparent, so the order in which we
re-parent replacements determines their final sibling order. To rebuild
`GetChildren()` exactly as it was before the swap we must:
* process ancestors before descendants, so each replacement's parent already
exists when we re-parent the replacement, and
* process siblings in their original `GetChildren()` order.
`swaps` is an array of `{ id, replacement, oldInstance }` entries. This sorts
the array in place (annotating each entry with `depth`/`siblingIndex`) and
returns it.
]]
local function orderSwaps(swaps)
for _, swap in swaps do
local depth = 0
local ancestor = swap.oldInstance.Parent
while ancestor ~= nil do
depth += 1
ancestor = ancestor.Parent
end
swap.depth = depth
local siblingIndex = 0
if swap.oldInstance.Parent ~= nil then
siblingIndex = table.find(swap.oldInstance.Parent:GetChildren(), swap.oldInstance) or 0
end
swap.siblingIndex = siblingIndex
end
table.sort(swaps, function(a, b)
if a.depth ~= b.depth then
return a.depth < b.depth
end
return a.siblingIndex < b.siblingIndex
end)
return swaps
end
return orderSwaps

View File

@@ -0,0 +1,57 @@
return function()
local orderSwaps = require(script.Parent.orderSwaps)
it("orders same-named siblings by their original GetChildren order", function()
local parent = Instance.new("Model")
local a1 = Instance.new("Part")
a1.Name = "a"
a1.Parent = parent
local a2 = Instance.new("Part")
a2.Name = "a"
a2.Parent = parent
local a3 = Instance.new("Part")
a3.Name = "a"
a3.Parent = parent
-- Input deliberately out of sibling order.
-- orderSwaps must restore the GetChildren() order.
local ordered = orderSwaps({
{ id = "3", oldInstance = a3 },
{ id = "1", oldInstance = a1 },
{ id = "2", oldInstance = a2 },
})
expect(ordered[1].oldInstance).to.equal(a1)
expect(ordered[2].oldInstance).to.equal(a2)
expect(ordered[3].oldInstance).to.equal(a3)
end)
it("orders ancestors before descendants", function()
local root = Instance.new("Model")
local child = Instance.new("Folder")
child.Parent = root
local grandchild = Instance.new("Part")
grandchild.Parent = child
local ordered = orderSwaps({
{ id = "grandchild", oldInstance = grandchild },
{ id = "child", oldInstance = child },
{ id = "root", oldInstance = root },
})
expect(ordered[1].oldInstance).to.equal(root)
expect(ordered[2].oldInstance).to.equal(child)
expect(ordered[3].oldInstance).to.equal(grandchild)
end)
it("returns a single swap unchanged", function()
local part = Instance.new("Part")
local ordered = orderSwaps({
{ id = "1", oldInstance = part },
})
expect(#ordered).to.equal(1)
expect(ordered[1].oldInstance).to.equal(part)
end)
end

View File

@@ -0,0 +1,16 @@
---
source: tests/tests/build.rs
expression: contents
---
<roblox version="4">
<Item class="Folder" referent="0">
<Properties>
<string name="Name">json_model_legacy_name</string>
</Properties>
<Item class="Folder" referent="1">
<Properties>
<string name="Name">Expected Name</string>
</Properties>
</Item>
</Item>
</roblox>

View File

@@ -1,23 +0,0 @@
---
source: tests/tests/build.rs
assertion_line: 109
expression: contents
---
<roblox version="4">
<Item class="DataModel" referent="0">
<Properties>
<string name="Name">model_json_name_input</string>
</Properties>
<Item class="Workspace" referent="1">
<Properties>
<string name="Name">Workspace</string>
<bool name="NeedsPivotMigration">false</bool>
</Properties>
<Item class="StringValue" referent="2">
<Properties>
<string name="Name">/Bar</string>
</Properties>
</Item>
</Item>
</Item>
</roblox>

View File

@@ -0,0 +1,27 @@
---
source: tests/tests/build.rs
expression: contents
---
<roblox version="4">
<Item class="Folder" referent="0">
<Properties>
<string name="Name">plugin_init</string>
</Properties>
<Item class="Script" referent="1">
<Properties>
<string name="Name">lua</string>
<token name="RunContext">3</token>
<string name="Source"><![CDATA[return "From folder/lua/init.plugin.lua"
]]></string>
</Properties>
</Item>
<Item class="Script" referent="2">
<Properties>
<string name="Name">luau</string>
<token name="RunContext">3</token>
<string name="Source"><![CDATA[return "From folder/luau/init.plugin.luau"
]]></string>
</Properties>
</Item>
</Item>
</roblox>

View File

@@ -1,20 +0,0 @@
---
source: tests/tests/build.rs
assertion_line: 108
expression: contents
---
<roblox version="4">
<Item class="Folder" referent="0">
<Properties>
<string name="Name">slugified_name_roundtrip</string>
</Properties>
<Item class="Script" referent="1">
<Properties>
<string name="Name">/Script</string>
<token name="RunContext">0</token>
<string name="Source"><![CDATA[print("Hello world!")
]]></string>
</Properties>
</Item>
</Item>
</roblox>

View File

@@ -0,0 +1,6 @@
{
"name": "json_model_legacy_name",
"tree": {
"$path": "folder"
}
}

View File

@@ -0,0 +1,4 @@
{
"Name": "Overridden Name",
"ClassName": "Folder"
}

View File

@@ -1,11 +0,0 @@
{
"name": "model_json_name_input",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -1,5 +0,0 @@
{
"name": "/Bar",
"className": "StringValue"
}

View File

@@ -0,0 +1,6 @@
{
"name": "plugin_init",
"tree": {
"$path": "folder"
}
}

View File

@@ -0,0 +1 @@
return "From folder/lua/init.plugin.lua"

View File

@@ -0,0 +1 @@
return "From folder/luau/init.plugin.luau"

View File

@@ -1,4 +0,0 @@
{
"name": "/Script"
}

View File

@@ -1,2 +0,0 @@
print("Hello world!")

View File

@@ -1,6 +0,0 @@
{
"name": "slugified_name_roundtrip",
"tree": {
"$path": "src"
}
}

View File

@@ -1,3 +0,0 @@
{
"name": "/Script"
}

View File

@@ -1 +0,0 @@
print("Hello world!")

View File

@@ -1,6 +1,5 @@
---
source: tests/rojo_test/syncback_util.rs
assertion_line: 101
expression: "String::from_utf8_lossy(&output.stdout)"
---
Writing default.project.json

View File

@@ -1,13 +0,0 @@
---
source: tests/rojo_test/syncback_util.rs
assertion_line: 101
expression: "String::from_utf8_lossy(&output.stdout)"
---
Writing default.project.json
Writing src/Camera.rbxm
Writing src/Terrain.rbxm
Writing src/_Folder/init.meta.json
Writing src/_Script.meta.json
Writing src/_Script.server.luau
Writing src
Writing src/_Folder

View File

@@ -1,9 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/foo.model.json
---
{
"name": "/Bar",
"className": "StringValue"
}

View File

@@ -0,0 +1,59 @@
---
source: tests/tests/syncback.rs
expression: default.project.json
---
{
"name": "SyncbackTest",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"TestPart": {
"$className": "Part",
"$properties": {
"Anchored": true,
"Color": {
"Color3uint8": [
0,
0,
255
]
}
}
},
"$properties": {
"EnableSLIMAvatars": {
"Enum": 0
},
"ImprovedAnimationConstraint": {
"Enum": 0
},
"ImprovedPhysicsReplication": {
"Enum": 0
},
"LayeredClothingCacheOptimizations": {
"Enum": 0
},
"MeshStreamingAndImprovedLods": {
"Enum": 0
},
"NextGenerationReplication": {
"Enum": 0
},
"PlayerScriptsUseInputActionSystem": {
"Enum": 0
},
"UseFixedSimulation": {
"Enum": 0
},
"UseNewLuauTypeSolver": "Disabled",
"ValidateEnabledProximityPrompt": {
"Enum": 0
}
},
"$attributes": {
"Rojo_Target_CurrentCamera": "302d573157260ee80a3baa32000003b5"
}
}
}
}

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Folder.model.json
---
{
"className": "Folder"
}

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Folder/init.meta.json
---
{
"name": "/Folder"
}

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script.meta.json
---
{
"name": "/Script"
}

View File

@@ -1,6 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script.server.luau
---
print("Hello world!")

View File

@@ -1,8 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script/init.meta.json
---
{
"name": "/Script"
}

View File

@@ -1,6 +0,0 @@
---
source: tests/tests/syncback.rs
assertion_line: 31
expression: src/_Script/init.server.luau
---
print("Hello world!")

View File

@@ -1,11 +0,0 @@
{
"name": "model_json_name",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -1,5 +0,0 @@
{
"name": "/Bar",
"className": "StringValue"
}

View File

@@ -0,0 +1,21 @@
{
"name": "SyncbackTest",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"TestPart": {
"$className": "Part",
"$properties": {
"Transparency": 1.0,
"Anchored": true,
"Color": [
1.0,
0.0,
0.0
]
}
}
}
}
}

View File

@@ -1,10 +0,0 @@
{
"name": "slugified_name",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"$path": "src"
}
}
}

View File

@@ -3,3 +3,4 @@ rojo = "rojo-rbx/rojo@7.5.1"
selene = "Kampfkarren/selene@0.29.0"
stylua = "JohnnyMorganz/stylua@2.1.0"
run-in-roblox = "rojo-rbx/run-in-roblox@0.3.0"
lune = "lune-org/lune@0.10.4"

View File

@@ -9,7 +9,6 @@ use std::{
};
use crate::{
git::SharedGitFilter,
message_queue::MessageQueue,
snapshot::{
apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree,
@@ -47,15 +46,11 @@ pub struct ChangeProcessor {
impl ChangeProcessor {
/// Spin up the ChangeProcessor, connecting it to the given tree, VFS, and
/// outbound message queue.
///
/// If `git_filter` is provided, it will be refreshed on every VFS event
/// to ensure newly changed files are acknowledged.
pub fn start(
tree: Arc<Mutex<RojoTree>>,
vfs: Arc<Vfs>,
message_queue: Arc<MessageQueue<AppliedPatchSet>>,
tree_mutation_receiver: Receiver<PatchSet>,
git_filter: Option<SharedGitFilter>,
) -> Self {
let (shutdown_sender, shutdown_receiver) = crossbeam_channel::bounded(1);
let vfs_receiver = vfs.event_receiver();
@@ -63,7 +58,6 @@ impl ChangeProcessor {
tree,
vfs,
message_queue,
git_filter,
};
let job_thread = jod_thread::Builder::new()
@@ -117,10 +111,6 @@ struct JobThreadContext {
/// Whenever changes are applied to the DOM, we should push those changes
/// into this message queue to inform any connected clients.
message_queue: Arc<MessageQueue<AppliedPatchSet>>,
/// Optional Git filter for --git-since mode. When set, will be refreshed
/// on every VFS event to ensure newly changed files are acknowledged.
git_filter: Option<SharedGitFilter>,
}
impl JobThreadContext {
@@ -170,14 +160,6 @@ impl JobThreadContext {
fn handle_vfs_event(&self, event: VfsEvent) {
log::trace!("Vfs event: {:?}", event);
// If we have a git filter, refresh it to pick up any new changes.
// This ensures that files modified during the session will be acknowledged.
if let Some(ref git_filter) = self.git_filter {
if let Err(err) = git_filter.refresh() {
log::warn!("Failed to refresh git filter: {:?}", err);
}
}
// Update the VFS immediately with the event.
self.vfs
.commit_event(&event)
@@ -218,7 +200,15 @@ impl JobThreadContext {
if let Some(instance) = tree.get_instance(id) {
if let Some(instigating_source) = &instance.metadata().instigating_source {
match instigating_source {
InstigatingSource::Path(path) => fs::remove_file(path).unwrap(),
InstigatingSource::Path(path) => {
if let Err(err) = fs::remove_file(path) {
log::error!(
"Failed to remove file {}: {}",
path.display(),
err
);
}
}
InstigatingSource::ProjectNode { .. } => {
log::warn!(
"Cannot remove instance {:?}, it's from a project file",
@@ -262,7 +252,13 @@ impl JobThreadContext {
match instigating_source {
InstigatingSource::Path(path) => {
if let Some(Variant::String(value)) = changed_value {
fs::write(path, value).unwrap();
if let Err(err) = fs::write(path, value) {
log::error!(
"Failed to write file {}: {}",
path.display(),
err
);
}
} else {
log::warn!("Cannot change Source to non-string value.");
}

View File

@@ -75,23 +75,28 @@ impl BuildCommand {
_ => unreachable!(),
};
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
log::trace!("Constructing in-memory filesystem");
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(self.watch);
let session = ServeSession::new(vfs, project_path, None)?;
let session = ServeSession::new(vfs, project_path)?;
let mut cursor = session.message_queue().cursor();
write_model(&session, &output_path, output_kind)?;
if self.watch {
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?;
loop {
let receiver = session.message_queue().subscribe(cursor);
let (new_cursor, _patch_set) = rt.block_on(receiver).unwrap();
let (new_cursor, _patch_set) = match rt.block_on(receiver) {
Ok(message) => message,
// The message queue was dropped, so there is nothing left
// to watch. Stop watching gracefully.
Err(_) => break,
};
cursor = new_cursor;
write_model(&session, &output_path, output_kind)?;

View File

@@ -18,10 +18,10 @@ pub struct FmtProjectCommand {
impl FmtProjectCommand {
pub fn run(self) -> anyhow::Result<()> {
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(false);
let base_path = resolve_path(&self.project);
let base_path = resolve_path(&self.project)?;
let project = Project::load_fuzzy(&vfs, &base_path)?
.context("A project file is required to run 'rojo fmt-project'")?;

View File

@@ -9,7 +9,7 @@ use std::{
io::{self, Write},
};
use anyhow::{bail, format_err};
use anyhow::{bail, format_err, Context};
use clap::Parser;
use fs_err as fs;
use fs_err::OpenOptions;
@@ -42,9 +42,9 @@ pub struct InitCommand {
impl InitCommand {
pub fn run(self) -> anyhow::Result<()> {
let template = self.kind.template();
let template = self.kind.template()?;
let base_path = resolve_path(&self.path);
let base_path = resolve_path(&self.path)?;
fs::create_dir_all(&base_path)?;
let canonical = fs::canonicalize(&base_path)?;
@@ -128,7 +128,7 @@ pub enum InitKind {
}
impl InitKind {
fn template(&self) -> InMemoryFs {
fn template(&self) -> anyhow::Result<InMemoryFs> {
let template_path = match self {
Self::Place => "place",
Self::Model => "model",
@@ -136,20 +136,24 @@ impl InitKind {
};
let snapshot: VfsSnapshot = bincode::deserialize(TEMPLATE_BINCODE)
.expect("Rojo's templates were not properly packed into Rojo's binary");
.context("Rojo's templates were not properly packed into Rojo's binary. This is a bug in Rojo; please file an issue.")?;
if let VfsSnapshot::Dir { mut children } = snapshot {
if let Some(template) = children.remove(template_path) {
let mut fs = InMemoryFs::new();
fs.load_snapshot("", template)
.expect("loading a template in memory should never fail");
fs
} else {
panic!("template for project type {:?} is missing", self)
}
} else {
panic!("Rojo's templates were packed as a file instead of a directory")
}
let VfsSnapshot::Dir { mut children } = snapshot else {
bail!("Rojo's templates were packed as a file instead of a directory. This is a bug in Rojo; please file an issue.");
};
let template = children.remove(template_path).ok_or_else(|| {
format_err!(
"The template for project type {:?} is missing. This is a bug in Rojo; please file an issue.",
self
)
})?;
let mut fs = InMemoryFs::new();
fs.load_snapshot("", template)
.context("Failed to load Rojo's bundled template into memory")?;
Ok(fs)
}
}

View File

@@ -12,6 +12,7 @@ mod upload;
use std::{borrow::Cow, env, path::Path, str::FromStr};
use anyhow::Context;
use clap::Parser;
use thiserror::Error;
@@ -125,10 +126,14 @@ pub enum Subcommand {
Syncback(SyncbackCommand),
}
pub(super) fn resolve_path(path: &Path) -> Cow<'_, Path> {
pub(super) fn resolve_path(path: &Path) -> anyhow::Result<Cow<'_, Path>> {
if path.is_absolute() {
Cow::Borrowed(path)
Ok(Cow::Borrowed(path))
} else {
Cow::Owned(env::current_dir().unwrap().join(path))
let current_dir = env::current_dir().context(
"Could not determine the current working directory. \
It may have been deleted, or Rojo may not have permission to access it.",
)?;
Ok(Cow::Owned(current_dir.join(path)))
}
}

View File

@@ -54,7 +54,7 @@ fn initialize_plugin() -> anyhow::Result<ServeSession> {
in_memory_fs.load_snapshot("/plugin", plugin_snapshot)?;
let vfs = Vfs::new(in_memory_fs);
Ok(ServeSession::new(vfs, "/plugin", None)?)
Ok(ServeSession::new(vfs, "/plugin")?)
}
fn install_plugin() -> anyhow::Result<()> {
@@ -98,5 +98,5 @@ fn uninstall_plugin() -> anyhow::Result<()> {
#[test]
fn plugin_initialize() {
assert!(initialize_plugin().is_ok())
let _ = initialize_plugin().unwrap();
}

View File

@@ -9,7 +9,7 @@ use clap::Parser;
use memofs::Vfs;
use termcolor::{BufferWriter, Color, ColorChoice, ColorSpec, WriteColor};
use crate::{git::GitFilter, serve_session::ServeSession, web::LiveServer};
use crate::{serve_session::ServeSession, web::LiveServer};
use super::{resolve_path, GlobalOptions};
@@ -32,39 +32,22 @@ pub struct ServeCommand {
#[clap(long)]
pub port: Option<u16>,
/// Only sync files that have changed since the given Git reference.
///
/// When this option is set, Rojo will only include files that have been
/// modified, added, or are untracked since the specified Git reference
/// (e.g., "HEAD", "main", a commit hash). This is useful for working with
/// large projects where you only want to sync your local changes.
///
/// Scripts that have not changed will still be acknowledged if modified
/// during the session, and all synced instances will have
/// ignoreUnknownInstances set to true to preserve descendants in Studio.
#[clap(long, value_name = "REF")]
pub git_since: Option<String>,
/// Extra `Host`/`Origin` values the server will accept, beyond localhost and
/// the bind address (for example a hostname like `mypc.lan`). Repeat the
/// option or comma-separate to allow several. When given, this overrides the
/// project's `serveAllowedHosts`. Listing any host also turns on Host/Origin
/// validation for binds where it is otherwise off (such as `0.0.0.0`).
#[clap(long, value_delimiter = ',')]
pub allowed_hosts: Vec<String>,
}
impl ServeCommand {
pub fn run(self, global: GlobalOptions) -> anyhow::Result<()> {
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
// Set up Git filter if --git-since was specified
let git_filter = if let Some(ref base_ref) = self.git_since {
let repo_root = GitFilter::find_repo_root(&project_path)?;
log::info!(
"Git filter enabled: only syncing files changed since '{}'",
base_ref
);
Some(Arc::new(GitFilter::new(repo_root, base_ref.clone(), &project_path)?))
} else {
None
};
let session = Arc::new(ServeSession::new(vfs, project_path, git_filter)?);
let session = Arc::new(ServeSession::new(vfs, project_path)?);
let ip = self
.address
@@ -76,27 +59,28 @@ impl ServeCommand {
.or_else(|| session.project_port())
.unwrap_or(DEFAULT_PORT);
// The CLI flag, when given, replaces the project's list rather than
// merging with it, matching how --address and --port override theirs.
let allowed_hosts = if self.allowed_hosts.is_empty() {
session.serve_allowed_hosts().to_vec()
} else {
self.allowed_hosts
};
let server = LiveServer::new(session);
let _ = show_start_message(ip, port, self.git_since.as_deref(), global.color.into());
server.start((ip, port).into());
server.start((ip, port).into(), allowed_hosts, || {
let _ = show_start_message(ip, port, global.color.into());
})?;
Ok(())
}
}
fn show_start_message(
bind_address: IpAddr,
port: u16,
git_since: Option<&str>,
color: ColorChoice,
) -> io::Result<()> {
fn show_start_message(bind_address: IpAddr, port: u16, color: ColorChoice) -> io::Result<()> {
let mut green = ColorSpec::new();
green.set_fg(Some(Color::Green)).set_bold(true);
let mut yellow = ColorSpec::new();
yellow.set_fg(Some(Color::Yellow)).set_bold(true);
let writer = BufferWriter::stdout(color);
let mut buffer = writer.buffer();
@@ -117,15 +101,27 @@ fn show_start_message(
buffer.set_color(&green)?;
writeln!(&mut buffer, "{}", port)?;
if let Some(base_ref) = git_since {
buffer.set_color(&ColorSpec::new())?;
write!(&mut buffer, " Mode: ")?;
buffer.set_color(&yellow)?;
writeln!(&mut buffer, "git-since ({})", base_ref)?;
}
writeln!(&mut buffer)?;
if !bind_address.is_loopback() {
let mut warning = ColorSpec::new();
warning.set_fg(Some(Color::Yellow)).set_bold(true);
buffer.set_color(&warning)?;
writeln!(
&mut buffer,
"WARNING: This server is bound to {address_string}, which is reachable from the \
network.\n\
The serve API is unauthenticated, so anyone who can reach {address_string}:{port} \
can read\n\
and modify your project's source. Prefer binding to localhost and tunneling (e.g. \
SSH,\n\
Tailscale, or WireGuard) when you need remote access."
)?;
buffer.set_color(&ColorSpec::new())?;
writeln!(&mut buffer)?;
}
buffer.set_color(&ColorSpec::new())?;
write!(&mut buffer, "Visit ")?;

View File

@@ -5,6 +5,7 @@ use std::{
path::{self, Path, PathBuf},
};
use anyhow::Context;
use clap::Parser;
use fs_err::File;
use memofs::Vfs;
@@ -71,14 +72,14 @@ pub struct SourcemapCommand {
impl SourcemapCommand {
pub fn run(self) -> anyhow::Result<()> {
let project_path = fs_err::canonicalize(resolve_path(&self.project))?;
let project_path = fs_err::canonicalize(resolve_path(&self.project)?)?;
log::trace!("Constructing filesystem with StdBackend");
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(self.watch);
log::trace!("Setting up session for sourcemap generation");
let session = ServeSession::new(vfs, project_path, None)?;
let session = ServeSession::new(vfs, project_path)?;
let mut cursor = session.message_queue().cursor();
let filter = if self.include_non_scripts {
@@ -100,11 +101,16 @@ impl SourcemapCommand {
if self.watch {
log::trace!("Setting up runtime for watch mode");
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?;
loop {
let receiver = session.message_queue().subscribe(cursor);
let (new_cursor, patch_set) = rt.block_on(receiver).unwrap();
let (new_cursor, patch_set) = match rt.block_on(receiver) {
Ok(message) => message,
// The message queue was dropped, so there is nothing left
// to watch. Stop watching gracefully.
Err(_) => break,
};
cursor = new_cursor;
if patch_set_affects_sourcemap(&session, &patch_set, filter) {

View File

@@ -54,17 +54,12 @@ pub struct SyncbackCommand {
/// If provided, the prompt for writing to the file system is skipped.
#[clap(long, short = 'y')]
pub non_interactive: bool,
/// If provided, forces syncback to use JSON model files instead of binary
/// .rbxm files for instances that would otherwise serialize as binary.
#[clap(long)]
pub dangerously_force_json: bool,
}
impl SyncbackCommand {
pub fn run(&self, global: GlobalOptions) -> anyhow::Result<()> {
let path_old = resolve_path(&self.project);
let path_new = resolve_path(&self.input);
let path_old = resolve_path(&self.project)?;
let path_new = resolve_path(&self.input)?;
let input_kind = FileKind::from_path(&path_new).context(UNKNOWN_INPUT_KIND_ERR)?;
let dom_start_timer = Instant::now();
@@ -74,11 +69,11 @@ impl SyncbackCommand {
dom_start_timer.elapsed().as_secs_f32()
);
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(false);
let project_start_timer = Instant::now();
let session_old = ServeSession::new(vfs, path_old.clone(), None)?;
let session_old = ServeSession::new(vfs, path_old.clone())?;
log::debug!(
"Finished opening project in {:0.02}s",
project_start_timer.elapsed().as_secs_f32()
@@ -109,7 +104,6 @@ impl SyncbackCommand {
&mut dom_old,
dom_new,
session_old.root_project(),
self.dangerously_force_json,
)?;
log::debug!(
"Syncback finished in {:.02}s!",

View File

@@ -38,11 +38,11 @@ pub struct UploadCommand {
impl UploadCommand {
pub fn run(self) -> Result<(), anyhow::Error> {
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
let session = ServeSession::new(vfs, project_path, None)?;
let session = ServeSession::new(vfs, project_path)?;
let tree = session.tree();
let inner_tree = tree.inner();

View File

@@ -1,380 +0,0 @@
//! Git integration for filtering files based on changes since a reference.
use std::{
collections::HashSet,
path::{Path, PathBuf},
process::Command,
sync::{Arc, RwLock},
};
use anyhow::{bail, Context};
/// A filter that tracks which files have been changed since a Git reference.
///
/// When active, only files that have been modified, added, or deleted according
/// to Git will be "acknowledged" and synced to Studio. This allows users to
/// work with large projects where they only want to sync their local changes.
///
/// Once a file is acknowledged (either initially or during the session), it
/// stays acknowledged for the entire session. This prevents files from being
/// deleted in Studio if their content is reverted to match the git reference.
#[derive(Debug)]
pub struct GitFilter {
/// The Git repository root directory.
repo_root: PathBuf,
/// The Git reference to compare against (e.g., "HEAD", "main", a commit hash).
base_ref: String,
/// Cache of paths that are currently different from the base ref according to git.
/// This is refreshed on every VFS event.
git_changed_paths: RwLock<HashSet<PathBuf>>,
/// Paths that have been acknowledged at any point during this session.
/// Once a path is added here, it stays acknowledged forever (for this session).
/// This prevents files from being deleted if their content is reverted.
session_acknowledged_paths: RwLock<HashSet<PathBuf>>,
}
impl GitFilter {
/// Creates a new GitFilter for the given repository root and base reference.
///
/// The `repo_root` should be the root of the Git repository (where .git is located).
/// The `base_ref` is the Git reference to compare against (e.g., "HEAD", "main").
/// The `project_path` is the path to the project being served - it will always be
/// acknowledged regardless of git status to ensure the project structure exists.
pub fn new(repo_root: PathBuf, base_ref: String, project_path: &Path) -> anyhow::Result<Self> {
let filter = Self {
repo_root,
base_ref,
git_changed_paths: RwLock::new(HashSet::new()),
session_acknowledged_paths: RwLock::new(HashSet::new()),
};
// Always acknowledge the project path and its directory so the project
// structure exists even when there are no git changes
filter.acknowledge_project_path(project_path);
// Initial refresh to populate the cache with git changes
filter.refresh()?;
Ok(filter)
}
/// Acknowledges the project path and its containing directory.
/// This ensures the project structure always exists regardless of git status.
fn acknowledge_project_path(&self, project_path: &Path) {
let mut session = self.session_acknowledged_paths.write().unwrap();
// Acknowledge the project path itself (might be a directory or .project.json file)
let canonical = project_path.canonicalize().unwrap_or_else(|_| project_path.to_path_buf());
session.insert(canonical.clone());
// Acknowledge all ancestor directories
let mut current = canonical.parent();
while let Some(parent) = current {
session.insert(parent.to_path_buf());
current = parent.parent();
}
// If it's a directory, also acknowledge default.project.json inside it
if project_path.is_dir() {
for name in &["default.project.json", "default.project.jsonc"] {
let project_file = project_path.join(name);
if let Ok(canonical_file) = project_file.canonicalize() {
session.insert(canonical_file);
} else {
session.insert(project_file);
}
}
}
// If it's a .project.json file, also acknowledge its parent directory
if let Some(parent) = project_path.parent() {
let parent_canonical = parent.canonicalize().unwrap_or_else(|_| parent.to_path_buf());
session.insert(parent_canonical);
}
log::debug!(
"GitFilter: acknowledged project path {} ({} paths total)",
project_path.display(),
session.len()
);
}
/// Finds the Git repository root for the given path.
pub fn find_repo_root(path: &Path) -> anyhow::Result<PathBuf> {
let output = Command::new("git")
.args(["rev-parse", "--show-toplevel"])
.current_dir(path)
.output()
.context("Failed to execute git rev-parse")?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
bail!("Failed to find Git repository root: {}", stderr.trim());
}
let root = String::from_utf8_lossy(&output.stdout)
.trim()
.to_string();
Ok(PathBuf::from(root))
}
/// Refreshes the cache of acknowledged paths by querying Git.
///
/// This should be called when files change to ensure newly modified files
/// are properly acknowledged. Once a path is acknowledged, it stays
/// acknowledged for the entire session (even if the file is reverted).
pub fn refresh(&self) -> anyhow::Result<()> {
let mut git_changed = HashSet::new();
// Get files changed since the base ref (modified, added, deleted)
let diff_output = Command::new("git")
.args(["diff", "--name-only", &self.base_ref])
.current_dir(&self.repo_root)
.output()
.context("Failed to execute git diff")?;
if !diff_output.status.success() {
let stderr = String::from_utf8_lossy(&diff_output.stderr);
bail!("git diff failed: {}", stderr.trim());
}
let diff_files = String::from_utf8_lossy(&diff_output.stdout);
let diff_count = diff_files.lines().filter(|l| !l.is_empty()).count();
if diff_count > 0 {
log::debug!("git diff found {} changed files", diff_count);
}
for line in diff_files.lines() {
if !line.is_empty() {
let path = self.repo_root.join(line);
log::trace!("git diff: acknowledging {}", path.display());
self.acknowledge_path(&path, &mut git_changed);
}
}
// Get untracked files (new files not yet committed)
let untracked_output = Command::new("git")
.args(["ls-files", "--others", "--exclude-standard"])
.current_dir(&self.repo_root)
.output()
.context("Failed to execute git ls-files")?;
if !untracked_output.status.success() {
let stderr = String::from_utf8_lossy(&untracked_output.stderr);
bail!("git ls-files failed: {}", stderr.trim());
}
let untracked_files = String::from_utf8_lossy(&untracked_output.stdout);
for line in untracked_files.lines() {
if !line.is_empty() {
let path = self.repo_root.join(line);
self.acknowledge_path(&path, &mut git_changed);
}
}
// Get staged files (files added to index but not yet committed)
let staged_output = Command::new("git")
.args(["diff", "--name-only", "--cached", &self.base_ref])
.current_dir(&self.repo_root)
.output()
.context("Failed to execute git diff --cached")?;
if staged_output.status.success() {
let staged_files = String::from_utf8_lossy(&staged_output.stdout);
for line in staged_files.lines() {
if !line.is_empty() {
let path = self.repo_root.join(line);
self.acknowledge_path(&path, &mut git_changed);
}
}
}
// Update the git changed paths cache
{
let mut cache = self.git_changed_paths.write().unwrap();
*cache = git_changed.clone();
}
// Merge newly changed paths into session acknowledged paths
// Once acknowledged, a path stays acknowledged for the entire session
{
let mut session = self.session_acknowledged_paths.write().unwrap();
for path in git_changed {
session.insert(path);
}
log::debug!(
"GitFilter refreshed: {} paths acknowledged in session",
session.len()
);
}
Ok(())
}
/// Acknowledges a path and all its ancestors, plus associated meta files.
fn acknowledge_path(&self, path: &Path, acknowledged: &mut HashSet<PathBuf>) {
// Canonicalize the path if possible, otherwise use as-is
let path = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
// Add the path itself
acknowledged.insert(path.clone());
// Add all ancestor directories
let mut current = path.parent();
while let Some(parent) = current {
acknowledged.insert(parent.to_path_buf());
current = parent.parent();
}
// Add associated meta files
self.acknowledge_meta_files(&path, acknowledged);
}
/// Acknowledges associated meta files for a given path.
fn acknowledge_meta_files(&self, path: &Path, acknowledged: &mut HashSet<PathBuf>) {
if let Some(file_name) = path.file_name().and_then(|n| n.to_str()) {
if let Some(parent) = path.parent() {
// For a file like "foo.lua", also acknowledge "foo.meta.json"
// Strip known extensions to get the base name
let base_name = strip_lua_extension(file_name);
let meta_path = parent.join(format!("{}.meta.json", base_name));
if let Ok(canonical) = meta_path.canonicalize() {
acknowledged.insert(canonical);
} else {
acknowledged.insert(meta_path);
}
// For init files, also acknowledge "init.meta.json" in the same directory
if file_name.starts_with("init.") {
let init_meta = parent.join("init.meta.json");
if let Ok(canonical) = init_meta.canonicalize() {
acknowledged.insert(canonical);
} else {
acknowledged.insert(init_meta);
}
}
}
}
}
/// Checks if a path is acknowledged (should be synced).
///
/// Returns `true` if the path or any of its descendants have been changed
/// at any point during this session. Once a file is acknowledged, it stays
/// acknowledged even if its content is reverted to match the git reference.
pub fn is_acknowledged(&self, path: &Path) -> bool {
let session = self.session_acknowledged_paths.read().unwrap();
// Try to canonicalize the path
let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
// Check if this exact path is acknowledged
if session.contains(&canonical) {
log::trace!("Path {} is directly acknowledged", path.display());
return true;
}
// Also check without canonicalization in case of path differences
if session.contains(path) {
log::trace!("Path {} is acknowledged (non-canonical)", path.display());
return true;
}
// For directories, check if any descendant is acknowledged
// This is done by checking if any acknowledged path starts with this path
for acknowledged in session.iter() {
if acknowledged.starts_with(&canonical) {
log::trace!(
"Path {} has acknowledged descendant {}",
path.display(),
acknowledged.display()
);
return true;
}
// Also check non-canonical
if acknowledged.starts_with(path) {
log::trace!(
"Path {} has acknowledged descendant {} (non-canonical)",
path.display(),
acknowledged.display()
);
return true;
}
}
log::trace!(
"Path {} is NOT acknowledged (canonical: {})",
path.display(),
canonical.display()
);
false
}
/// Returns the base reference being compared against.
pub fn base_ref(&self) -> &str {
&self.base_ref
}
/// Returns the repository root path.
pub fn repo_root(&self) -> &Path {
&self.repo_root
}
/// Explicitly acknowledges a path and all its ancestors.
/// This is useful for ensuring certain paths are always synced regardless of git status.
pub fn force_acknowledge(&self, path: &Path) {
let mut acknowledged = HashSet::new();
self.acknowledge_path(path, &mut acknowledged);
let mut session = self.session_acknowledged_paths.write().unwrap();
for p in acknowledged {
session.insert(p);
}
}
}
/// Strips Lua-related extensions from a file name to get the base name.
fn strip_lua_extension(file_name: &str) -> &str {
const EXTENSIONS: &[&str] = &[
".server.luau",
".server.lua",
".client.luau",
".client.lua",
".luau",
".lua",
];
for ext in EXTENSIONS {
if let Some(base) = file_name.strip_suffix(ext) {
return base;
}
}
// If no Lua extension, try to strip the regular extension
file_name
.rsplit_once('.')
.map(|(base, _)| base)
.unwrap_or(file_name)
}
/// A wrapper around GitFilter that can be shared across threads.
pub type SharedGitFilter = Arc<GitFilter>;
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_strip_lua_extension() {
assert_eq!(strip_lua_extension("foo.server.lua"), "foo");
assert_eq!(strip_lua_extension("foo.client.luau"), "foo");
assert_eq!(strip_lua_extension("foo.lua"), "foo");
assert_eq!(strip_lua_extension("init.server.lua"), "init");
assert_eq!(strip_lua_extension("bar.txt"), "bar");
assert_eq!(strip_lua_extension("noextension"), "noextension");
}
}

View File

@@ -48,3 +48,61 @@ impl<'de> Deserialize<'de> for Glob {
Glob::new(&glob).map_err(D::Error::custom)
}
}
/// A glob with optional gitignore-style negation. A leading `!` marks the
/// pattern as a negation (re-includes paths that an earlier rule excluded).
/// To match a literal `!` at the start of a pattern, escape it with `\!`.
#[derive(Debug, Clone)]
pub struct IgnorableGlob {
glob: Glob,
negated: bool,
raw: String,
}
impl IgnorableGlob {
pub fn new(pattern: &str) -> Result<Self, Error> {
let (negated, body) = if let Some(rest) = pattern.strip_prefix('!') {
(true, rest)
} else if pattern.starts_with(r"\!") {
(false, &pattern[1..])
} else {
(false, pattern)
};
Ok(IgnorableGlob {
glob: Glob::new(body)?,
negated,
raw: pattern.to_owned(),
})
}
pub fn is_match<P: AsRef<Path>>(&self, path: P) -> bool {
self.glob.is_match(path)
}
pub fn is_negation(&self) -> bool {
self.negated
}
}
impl PartialEq for IgnorableGlob {
fn eq(&self, other: &Self) -> bool {
self.negated == other.negated && self.glob == other.glob
}
}
impl Eq for IgnorableGlob {}
impl Serialize for IgnorableGlob {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_str(&self.raw)
}
}
impl<'de> Deserialize<'de> for IgnorableGlob {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let pattern = String::deserialize(deserializer)?;
IgnorableGlob::new(&pattern).map_err(D::Error::custom)
}
}

View File

@@ -9,7 +9,6 @@ mod tree_view;
mod auth_cookie;
mod change_processor;
mod git;
mod glob;
mod json;
mod lua_ast;
@@ -29,7 +28,6 @@ mod web;
// TODO: Work out what we should expose publicly
pub use git::{GitFilter, SharedGitFilter};
pub use project::*;
pub use rojo_ref::*;
pub use session_id::SessionId;

View File

@@ -12,7 +12,8 @@ use serde::{Deserialize, Serialize};
use thiserror::Error;
use crate::{
glob::Glob, json, resolution::UnresolvedValue, snapshot::SyncRule, syncback::SyncbackRules,
glob::IgnorableGlob, json, resolution::UnresolvedValue, snapshot::SyncRule,
syncback::SyncbackRules,
};
/// Represents 'default' project names that act as `init` files
@@ -105,6 +106,15 @@ pub struct Project {
#[serde(skip_serializing_if = "Option::is_none")]
pub serve_address: Option<IpAddr>,
/// Additional `Host`/`Origin` header values that `rojo serve` will accept
/// beyond `localhost` and the bind address, such as a hostname like
/// `mypc.lan` used to reach a network-exposed server by name. Listing any
/// host also turns on `Host`/`Origin` validation for binds where it is
/// otherwise off (such as `0.0.0.0`). The `--allowed-hosts` CLI option
/// overrides this field when provided.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub serve_allowed_hosts: Vec<String>,
/// Determines if Rojo should emit scripts with the appropriate `RunContext`
/// for `*.client.lua` and `*.server.lua` files in the project instead of
/// using `Script` and `LocalScript` Instances.
@@ -114,7 +124,7 @@ pub struct Project {
/// A list of globs, relative to the folder the project file is in, that
/// match files that should be excluded if Rojo encounters them.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub glob_ignore_paths: Vec<Glob>,
pub glob_ignore_paths: Vec<IgnorableGlob>,
/// A list of rules for syncback with this project file.
#[serde(skip_serializing_if = "Option::is_none")]
@@ -593,4 +603,90 @@ mod test {
assert!(project.sync_rules[0].include.is_match("data.data.json"));
assert!(project.sync_rules[1].include.is_match("init.module.lua"));
}
#[test]
fn project_with_serve_allowed_hosts() {
let project_json = r#"{
"name": "TestProject",
"tree": { "$path": "src" },
"serveAllowedHosts": ["mypc.lan", "192.168.1.5"]
}"#;
let project = Project::load_from_slice(
project_json.as_bytes(),
PathBuf::from("/test/default.project.json"),
None,
)
.expect("Failed to parse project with serveAllowedHosts");
assert_eq!(project.serve_allowed_hosts, vec!["mypc.lan", "192.168.1.5"]);
}
#[test]
fn project_without_serve_allowed_hosts_defaults_to_empty() {
let project_json = r#"{
"name": "TestProject",
"tree": { "$path": "src" }
}"#;
let project = Project::load_from_slice(
project_json.as_bytes(),
PathBuf::from("/test/default.project.json"),
None,
)
.expect("Failed to parse project");
assert!(project.serve_allowed_hosts.is_empty());
}
#[test]
fn glob_ignore_paths_negation() {
let project_json = r#"{
"name": "TestProject",
"tree": { "$path": "src" },
"globIgnorePaths": [
"**/*.spec.lua",
"!keep.spec.lua",
"\\!literal.lua"
]
}"#;
let project = Project::load_from_slice(
project_json.as_bytes(),
PathBuf::from("/test/default.project.json"),
None,
)
.expect("project should parse");
let paths = &project.glob_ignore_paths;
assert_eq!(paths.len(), 3);
assert!(!paths[0].is_negation());
assert!(paths[0].is_match("foo.spec.lua"));
assert!(paths[1].is_negation());
assert!(paths[1].is_match("keep.spec.lua"));
// `\!literal.lua` should match a file literally named `!literal.lua`,
// not be parsed as a negation.
assert!(!paths[2].is_negation());
assert!(paths[2].is_match("!literal.lua"));
let rules: Vec<_> = paths
.iter()
.map(|g| crate::snapshot::PathIgnoreRule {
base_path: PathBuf::from("/test"),
glob: g.clone(),
})
.collect();
assert!(crate::snapshot::is_path_ignored(
&rules,
"/test/foo.spec.lua"
));
assert!(!crate::snapshot::is_path_ignored(
&rules,
"/test/keep.spec.lua"
));
assert!(!crate::snapshot::is_path_ignored(&rules, "/test/plain.lua"));
}
}

View File

@@ -13,7 +13,6 @@ use thiserror::Error;
use crate::{
change_processor::ChangeProcessor,
git::SharedGitFilter,
message_queue::MessageQueue,
project::{Project, ProjectError},
session_id::SessionId,
@@ -95,14 +94,7 @@ impl ServeSession {
/// The project file is expected to be loaded out-of-band since it's
/// currently loaded from the filesystem directly instead of through the
/// in-memory filesystem layer.
///
/// If `git_filter` is provided, only files that have changed since the
/// specified Git reference will be synced.
pub fn new<P: AsRef<Path>>(
vfs: Vfs,
start_path: P,
git_filter: Option<SharedGitFilter>,
) -> Result<Self, ServeSessionError> {
pub fn new<P: AsRef<Path>>(vfs: Vfs, start_path: P) -> Result<Self, ServeSessionError> {
let start_path = start_path.as_ref();
let start_time = Instant::now();
@@ -110,28 +102,12 @@ impl ServeSession {
let root_project = Project::load_initial_project(&vfs, start_path)?;
// If git filter is active, ensure the project file location is acknowledged
// This is necessary so the project structure exists even with no git changes
if let Some(ref filter) = git_filter {
filter.force_acknowledge(start_path);
filter.force_acknowledge(&root_project.file_location);
filter.force_acknowledge(root_project.folder_location());
log::debug!(
"Force acknowledged project at {}",
root_project.file_location.display()
);
}
let mut tree = RojoTree::new(InstanceSnapshot::new());
let root_id = tree.get_root_id();
let instance_context = match &git_filter {
Some(filter) => {
InstanceContext::with_git_filter(root_project.emit_legacy_scripts, Arc::clone(filter))
}
None => InstanceContext::with_emit_legacy_scripts(root_project.emit_legacy_scripts),
};
let instance_context =
InstanceContext::with_emit_legacy_scripts(root_project.emit_legacy_scripts);
log::trace!("Generating snapshot of instances from VFS");
let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?;
@@ -157,7 +133,6 @@ impl ServeSession {
Arc::clone(&vfs),
Arc::clone(&message_queue),
tree_mutation_receiver,
git_filter,
);
Ok(Self {
@@ -232,6 +207,10 @@ impl ServeSession {
self.root_project.serve_address
}
pub fn serve_allowed_hosts(&self) -> &[String] {
&self.root_project.serve_allowed_hosts
}
pub fn root_dir(&self) -> &Path {
self.root_project.folder_location()
}

View File

@@ -8,8 +8,7 @@ use anyhow::Context;
use serde::{Deserialize, Serialize};
use crate::{
git::SharedGitFilter,
glob::Glob,
glob::{Glob, IgnorableGlob},
path_serializer,
project::ProjectNode,
snapshot_middleware::{emit_legacy_scripts_default, Middleware},
@@ -71,12 +70,6 @@ pub struct InstanceMetadata {
/// A schema provided via a JSON file, if one exists. Will be `None` for
/// all non-JSON middleware.
pub schema: Option<String>,
/// A custom name specified via meta.json or model.json files. If present,
/// this name will be used for the instance while the filesystem name will
/// be slugified to remove illegal characters.
#[serde(skip_serializing_if = "Option::is_none")]
pub specified_name: Option<String>,
}
impl InstanceMetadata {
@@ -89,7 +82,6 @@ impl InstanceMetadata {
specified_id: None,
middleware: None,
schema: None,
specified_name: None,
}
}
@@ -138,13 +130,6 @@ impl InstanceMetadata {
pub fn schema(self, schema: Option<String>) -> Self {
Self { schema, ..self }
}
pub fn specified_name(self, specified_name: Option<String>) -> Self {
Self {
specified_name,
..self
}
}
}
impl Default for InstanceMetadata {
@@ -153,27 +138,13 @@ impl Default for InstanceMetadata {
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct InstanceContext {
#[serde(skip_serializing_if = "Vec::is_empty")]
pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
pub emit_legacy_scripts: bool,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub sync_rules: Vec<SyncRule>,
/// Optional Git filter for --git-since mode. When set, only files that have
/// changed since the specified Git reference will be synced.
#[serde(skip)]
pub git_filter: Option<SharedGitFilter>,
}
impl PartialEq for InstanceContext {
fn eq(&self, other: &Self) -> bool {
// Note: git_filter is intentionally excluded from comparison
// since it's runtime state, not configuration
self.path_ignore_rules == other.path_ignore_rules
&& self.emit_legacy_scripts == other.emit_legacy_scripts
&& self.sync_rules == other.sync_rules
}
}
impl InstanceContext {
@@ -182,7 +153,6 @@ impl InstanceContext {
path_ignore_rules: Arc::new(Vec::new()),
emit_legacy_scripts: emit_legacy_scripts_default().unwrap(),
sync_rules: Vec::new(),
git_filter: None,
}
}
@@ -195,36 +165,6 @@ impl InstanceContext {
}
}
/// Creates a new InstanceContext with a Git filter for --git-since mode.
pub fn with_git_filter(
emit_legacy_scripts: Option<bool>,
git_filter: SharedGitFilter,
) -> Self {
Self {
git_filter: Some(git_filter),
..Self::with_emit_legacy_scripts(emit_legacy_scripts)
}
}
/// Sets the Git filter for this context.
pub fn set_git_filter(&mut self, git_filter: Option<SharedGitFilter>) {
self.git_filter = git_filter;
}
/// Returns true if the given path should be acknowledged (synced).
/// If no git filter is set, all paths are acknowledged.
pub fn is_path_acknowledged(&self, path: &Path) -> bool {
match &self.git_filter {
Some(filter) => filter.is_acknowledged(path),
None => true,
}
}
/// Returns true if a git filter is active.
pub fn has_git_filter(&self) -> bool {
self.git_filter.is_some()
}
/// Extend the list of ignore rules in the context with the given new rules.
pub fn add_path_ignore_rules<I>(&mut self, new_rules: I)
where
@@ -282,18 +222,37 @@ pub struct PathIgnoreRule {
pub base_path: PathBuf,
/// The actual glob that can be matched against the input path.
pub glob: Glob,
pub glob: IgnorableGlob,
}
impl PathIgnoreRule {
pub fn passes<P: AsRef<Path>>(&self, path: P) -> bool {
pub fn matches<P: AsRef<Path>>(&self, path: P) -> bool {
let path = path.as_ref();
match path.strip_prefix(&self.base_path) {
Ok(suffix) => !self.glob.is_match(suffix),
Err(_) => true,
Ok(suffix) => self.glob.is_match(suffix),
Err(_) => false,
}
}
pub fn is_negation(&self) -> bool {
self.glob.is_negation()
}
}
/// Evaluates an ordered list of [`PathIgnoreRule`]s against a path using
/// gitignore-style "last match wins" semantics: a path is ignored if the last
/// rule whose pattern matches it is non-negated. Paths matched by no rule are
/// not ignored.
pub fn is_path_ignored<P: AsRef<Path>>(rules: &[PathIgnoreRule], path: P) -> bool {
let path = path.as_ref();
let mut ignored = false;
for rule in rules {
if rule.matches(path) {
ignored = !rule.is_negation();
}
}
ignored
}
/// Represents where a particular Instance or InstanceSnapshot came from.

View File

@@ -109,17 +109,8 @@ pub fn syncback_csv<'sync>(
if !meta.is_empty() {
let parent = snapshot.path.parent_err()?;
let meta_stem = snapshot.middleware
.and_then(|mw| {
let ext = format!(".{}", crate::syncback::extension_for_middleware(mw));
snapshot.path.file_name()
.and_then(|n| n.to_str())
.and_then(|s| s.strip_suffix(ext.as_str()))
.map(str::to_owned)
})
.unwrap_or_else(|| new_inst.name.clone());
fs_snapshot.add_file(
parent.join(format!("{meta_stem}.meta.json")),
parent.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
)
}
@@ -204,7 +195,7 @@ struct LocalizationEntry<'a> {
/// https://github.com/BurntSushi/rust-csv/issues/151
///
/// This function operates in one step in order to minimize data-copying.
fn convert_localization_csv(contents: &[u8]) -> Result<String, csv::Error> {
fn convert_localization_csv(contents: &[u8]) -> anyhow::Result<String> {
let mut reader = csv::Reader::from_reader(contents);
let headers = reader.headers()?.clone();
@@ -246,7 +237,7 @@ fn convert_localization_csv(contents: &[u8]) -> Result<String, csv::Error> {
}
let encoded =
serde_json::to_string(&entries).expect("Could not encode JSON for localization table");
serde_json::to_string(&entries).context("Could not encode JSON for localization table")?;
Ok(encoded)
}

View File

@@ -7,8 +7,10 @@ use anyhow::Context;
use memofs::{DirEntry, Vfs};
use crate::{
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
syncback::{hash_instance, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
snapshot::{
is_path_ignored, InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource,
},
syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
};
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs};
@@ -41,12 +43,8 @@ pub fn snapshot_dir_no_meta(
path: &Path,
name: &str,
) -> anyhow::Result<Option<InstanceSnapshot>> {
let passes_filter_rules = |child: &DirEntry| {
context
.path_ignore_rules
.iter()
.all(|rule| rule.passes(child.path()))
};
let passes_filter_rules =
|child: &DirEntry| !is_path_ignored(&context.path_ignore_rules, child.path());
let mut snapshot_children = Vec::new();
@@ -74,6 +72,8 @@ pub fn snapshot_dir_no_meta(
normalized_path.join("init.server.luau"),
normalized_path.join("init.client.lua"),
normalized_path.join("init.client.luau"),
normalized_path.join("init.plugin.lua"),
normalized_path.join("init.plugin.luau"),
normalized_path.join("init.csv"),
];
@@ -134,39 +134,12 @@ pub fn syncback_dir_no_meta<'sync>(
let mut children = Vec::new();
let mut removed_children = Vec::new();
// Build the old child map early so it can be used for deduplication below.
let mut old_child_map = HashMap::new();
if let Some(old_inst) = snapshot.old_inst() {
for child in old_inst.children() {
let inst = snapshot.get_old_instance(*child).unwrap();
old_child_map.insert(inst.name(), inst);
}
}
// Enforce unique filesystem names. Uses actual on-disk names for existing
// children and resolved names (with init-prefix) for new ones.
let mut fs_child_names = HashSet::with_capacity(new_inst.children().len());
// We have to enforce unique child names for the file system.
let mut child_names = HashSet::with_capacity(new_inst.children().len());
let mut duplicate_set = HashSet::new();
for child_ref in new_inst.children() {
let child = snapshot.get_new_instance(*child_ref).unwrap();
let fs_name = old_child_map
.get(child.name.as_str())
.and_then(|old| old.metadata().relevant_paths.first())
.and_then(|p| p.file_name())
.and_then(|n| n.to_str())
.map(|s| s.to_lowercase())
.unwrap_or_else(|| {
let slug = slugify_name(&child.name);
let slug_lower = slug.to_lowercase();
// Mirror name_for_inst's init-prefix.
if slug_lower == "init" {
format!("_{slug_lower}")
} else {
slug_lower
}
});
if !fs_child_names.insert(fs_name) {
if !child_names.insert(child.name.to_lowercase()) {
duplicate_set.insert(child.name.as_str());
}
}
@@ -180,7 +153,13 @@ pub fn syncback_dir_no_meta<'sync>(
anyhow::bail!("Instance has more than 25 children with duplicate names");
}
if snapshot.old_inst().is_some() {
if let Some(old_inst) = snapshot.old_inst() {
let mut old_child_map = HashMap::with_capacity(old_inst.children().len());
for child in old_inst.children() {
let inst = snapshot.get_old_instance(*child).unwrap();
old_child_map.insert(inst.name(), inst);
}
for new_child_ref in new_inst.children() {
let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();
if let Some(old_child) = old_child_map.remove(new_child.name.as_str()) {
@@ -246,12 +225,6 @@ pub fn syncback_dir_no_meta<'sync>(
mod test {
use super::*;
use std::path::PathBuf;
use crate::{
snapshot::{InstanceMetadata, InstanceSnapshot},
Project, RojoTree, SyncbackData, SyncbackSnapshot,
};
use memofs::{InMemoryFs, VfsSnapshot};
#[test]
@@ -288,237 +261,4 @@ mod test {
insta::assert_yaml_snapshot!(instance_snapshot);
}
fn make_project() -> Project {
serde_json::from_str(r#"{"tree": {"$className": "DataModel"}}"#).unwrap()
}
fn make_vfs() -> Vfs {
let mut imfs = InMemoryFs::new();
imfs.load_snapshot("/root", VfsSnapshot::empty_dir()).unwrap();
Vfs::new(imfs)
}
/// Two children whose Roblox names are identical when lowercased ("Alpha"
/// and "alpha") but live at different filesystem paths because of the
/// `name` property ("Beta/" and "Alpha/" respectively). The dedup check
/// must use the actual filesystem paths, not the raw Roblox names, to
/// avoid a false-positive duplicate error.
#[test]
fn syncback_no_false_duplicate_with_name_prop() {
use rbx_dom_weak::{InstanceBuilder, WeakDom};
// Old child A: Roblox name "Alpha", on disk at "/root/Beta"
// (name property maps "Alpha" → "Beta" on the filesystem)
let old_child_a = InstanceSnapshot::new()
.name("Alpha")
.class_name("Folder")
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root/Beta"))
.relevant_paths(vec![PathBuf::from("/root/Beta")]),
);
// Old child B: Roblox name "alpha", on disk at "/root/Alpha"
let old_child_b = InstanceSnapshot::new()
.name("alpha")
.class_name("Folder")
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root/Alpha"))
.relevant_paths(vec![PathBuf::from("/root/Alpha")]),
);
let old_parent = InstanceSnapshot::new()
.name("Parent")
.class_name("Folder")
.children(vec![old_child_a, old_child_b])
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root"))
.relevant_paths(vec![PathBuf::from("/root")]),
);
let old_tree = RojoTree::new(old_parent);
// New state: same two children in Roblox.
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
let new_parent = new_tree.insert(
new_tree.root_ref(),
InstanceBuilder::new("Folder").with_name("Parent"),
);
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Alpha"));
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("alpha"));
let vfs = make_vfs();
let project = make_project();
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
let snapshot = SyncbackSnapshot {
data,
old: Some(old_tree.get_root_id()),
new: new_parent,
path: PathBuf::from("/root"),
middleware: None,
};
let result = syncback_dir_no_meta(&snapshot);
assert!(
result.is_ok(),
"should not error when two children have the same lowercased Roblox \
name but map to distinct filesystem paths: {result:?}",
);
}
/// Two completely new children with the same non-init name would produce
/// the same filesystem entry and must be detected as a duplicate.
#[test]
fn syncback_detects_sibling_duplicate_names() {
use rbx_dom_weak::{InstanceBuilder, WeakDom};
let old_parent = InstanceSnapshot::new()
.name("Parent")
.class_name("Folder")
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root"))
.relevant_paths(vec![PathBuf::from("/root")]),
);
let old_tree = RojoTree::new(old_parent);
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
let new_parent = new_tree.insert(
new_tree.root_ref(),
InstanceBuilder::new("Folder").with_name("Parent"),
);
// "Foo" is not a reserved name but two siblings named "Foo" still
// collide on disk.
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo"));
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo"));
let vfs = make_vfs();
let project = make_project();
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
let snapshot = SyncbackSnapshot {
data,
old: Some(old_tree.get_root_id()),
new: new_parent,
path: PathBuf::from("/root"),
middleware: None,
};
let result = syncback_dir_no_meta(&snapshot);
assert!(
result.is_err(),
"should error when two new children would produce the same filesystem name",
);
}
/// A new child named "Init" (as a ModuleScript) would naively become
/// "Init.luau", which case-insensitively matches the parent's reserved
/// "init.luau". Syncback must resolve this automatically by prefixing the
/// filesystem name with '_' (→ "_Init.luau") rather than erroring.
#[test]
fn syncback_resolves_init_name_conflict() {
use rbx_dom_weak::{InstanceBuilder, WeakDom};
let old_parent = InstanceSnapshot::new()
.name("Parent")
.class_name("Folder")
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root"))
.relevant_paths(vec![PathBuf::from("/root")]),
);
let old_tree = RojoTree::new(old_parent);
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
let new_parent = new_tree.insert(
new_tree.root_ref(),
InstanceBuilder::new("Folder").with_name("Parent"),
);
new_tree.insert(
new_parent,
InstanceBuilder::new("ModuleScript").with_name("Init"),
);
let vfs = make_vfs();
let project = make_project();
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
let snapshot = SyncbackSnapshot {
data,
old: Some(old_tree.get_root_id()),
new: new_parent,
path: PathBuf::from("/root"),
middleware: None,
};
let result = syncback_dir_no_meta(&snapshot);
assert!(
result.is_ok(),
"should resolve init-name conflict by prefixing '_', not error: {result:?}",
);
// The child should have been placed at "_Init.luau", not "Init.luau".
let child_file_name = result
.unwrap()
.children
.into_iter()
.next()
.and_then(|c| c.path.file_name().map(|n| n.to_string_lossy().into_owned()))
.unwrap_or_default();
assert!(
child_file_name.starts_with('_'),
"child filesystem name should start with '_' to avoid init collision, \
got: {child_file_name}",
);
}
/// A child whose filesystem name is stored with a slugified prefix (e.g.
/// "_Init") must NOT be blocked — only the bare "init" stem is reserved.
#[test]
fn syncback_allows_slugified_init_name() {
use rbx_dom_weak::{InstanceBuilder, WeakDom};
// Existing child: on disk as "_Init" (slugified from a name with an
// illegal character), its stem is "_init" which is not reserved.
let old_child = InstanceSnapshot::new()
.name("Init")
.class_name("Folder")
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root/_Init"))
.relevant_paths(vec![PathBuf::from("/root/_Init")]),
);
let old_parent = InstanceSnapshot::new()
.name("Parent")
.class_name("Folder")
.children(vec![old_child])
.metadata(
InstanceMetadata::new()
.instigating_source(PathBuf::from("/root"))
.relevant_paths(vec![PathBuf::from("/root")]),
);
let old_tree = RojoTree::new(old_parent);
let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT"));
let new_parent = new_tree.insert(
new_tree.root_ref(),
InstanceBuilder::new("Folder").with_name("Parent"),
);
new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Init"));
let vfs = make_vfs();
let project = make_project();
let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project);
let snapshot = SyncbackSnapshot {
data,
old: Some(old_tree.get_root_id()),
new: new_parent,
path: PathBuf::from("/root"),
middleware: None,
};
let result = syncback_dir_no_meta(&snapshot);
assert!(
result.is_ok(),
"should allow a child whose filesystem name is slugified away from \
the reserved 'init' stem: {result:?}",
);
}
}

View File

@@ -35,14 +35,20 @@ pub fn snapshot_json_model(
format!("File is not a valid JSON model: {}", path.display())
})?;
// If the JSON has a name property, preserve it in metadata for syncback
let specified_name = instance.name.clone();
if let Some(top_level_name) = &instance.name {
let new_name = format!("{}.model.json", top_level_name);
// Use the name from JSON if present, otherwise fall back to filename-derived name
if instance.name.is_none() {
instance.name = Some(name.to_owned());
log::warn!(
"Model at path {} had a top-level Name field. \
This field has been ignored since Rojo 6.0.\n\
Consider removing this field and renaming the file to {}.",
new_name,
path.display()
);
}
instance.name = Some(name.to_owned());
let id = instance.id.take().map(RojoRef::new);
let schema = instance.schema.take();
@@ -56,8 +62,7 @@ pub fn snapshot_json_model(
.relevant_paths(vec![vfs.canonicalize(path)?])
.context(context)
.specified_id(id)
.schema(schema)
.specified_name(specified_name);
.schema(schema);
Ok(Some(snapshot))
}
@@ -76,7 +81,6 @@ pub fn syncback_json_model<'sync>(
// schemas will ever exist in one project for it to matter, but it
// could have a performance cost.
model.schema = old_inst.metadata().schema.clone();
model.name = old_inst.metadata().specified_name.clone();
}
Ok(SyncbackReturn {

View File

@@ -158,17 +158,8 @@ pub fn syncback_lua<'sync>(
if !meta.is_empty() {
let parent_location = snapshot.path.parent_err()?;
let meta_stem = snapshot.middleware
.and_then(|mw| {
let ext = format!(".{}", crate::syncback::extension_for_middleware(mw));
snapshot.path.file_name()
.and_then(|n| n.to_str())
.and_then(|s| s.strip_suffix(ext.as_str()))
.map(str::to_owned)
})
.unwrap_or_else(|| snapshot.new_inst().name.clone());
fs_snapshot.add_file(
parent_location.join(format!("{meta_stem}.meta.json")),
parent_location.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
);
}
@@ -191,6 +182,7 @@ pub fn syncback_lua_init<'sync>(
ScriptType::Server => "init.server.luau",
ScriptType::Client => "init.client.luau",
ScriptType::Module => "init.luau",
ScriptType::Plugin => "init.plugin.luau",
_ => anyhow::bail!("syncback is not yet implemented for {script_type:?}"),
});

View File

@@ -10,10 +10,7 @@ use rbx_dom_weak::{
use serde::{Deserialize, Serialize};
use crate::{
json,
resolution::UnresolvedValue,
snapshot::InstanceSnapshot,
syncback::{validate_file_name, SyncbackSnapshot},
json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot,
RojoRef,
};
@@ -39,9 +36,6 @@ pub struct AdjacentMetadata {
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub attributes: IndexMap<String, UnresolvedValue>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)]
pub path: PathBuf,
}
@@ -150,31 +144,6 @@ impl AdjacentMetadata {
}
}
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// Write name when the filesystem path doesn't match the
// instance name (invalid chars or init-prefix).
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
let fs_stem = path
.file_name()
.and_then(|n| n.to_str())
.map(|s| s.split('.').next().unwrap_or(s))
.unwrap_or("");
if validate_file_name(instance_name).is_err()
|| fs_stem != instance_name.as_str()
{
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances {
Some(true)
@@ -186,7 +155,6 @@ impl AdjacentMetadata {
path,
id: None,
schema,
name,
}))
}
@@ -245,26 +213,11 @@ impl AdjacentMetadata {
Ok(())
}
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
self.apply_ignore_unknown_instances(snapshot);
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(())
}
@@ -273,13 +226,11 @@ impl AdjacentMetadata {
///
/// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None
/// - `name` is None
#[inline]
pub fn is_empty(&self) -> bool {
self.attributes.is_empty()
&& self.properties.is_empty()
&& self.ignore_unknown_instances.is_none()
&& self.name.is_none()
}
// TODO: Add method to allow selectively applying parts of metadata and
@@ -311,9 +262,6 @@ pub struct DirectoryMetadata {
#[serde(skip_serializing_if = "Option::is_none")]
pub class_name: Option<Ustr>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)]
pub path: PathBuf,
}
@@ -424,30 +372,6 @@ impl DirectoryMetadata {
}
}
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// Write name when the directory name doesn't match the
// instance name (invalid chars or init-prefix).
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
let fs_name = path
.file_name()
.and_then(|n| n.to_str())
.unwrap_or("");
if validate_file_name(instance_name).is_err()
|| fs_name != instance_name.as_str()
{
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances {
Some(true)
@@ -460,7 +384,6 @@ impl DirectoryMetadata {
path,
id: None,
schema,
name,
}))
}
@@ -470,7 +393,6 @@ impl DirectoryMetadata {
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(())
}
@@ -542,33 +464,17 @@ impl DirectoryMetadata {
snapshot.metadata.schema = self.schema.take();
Ok(())
}
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
/// Returns whether the metadata is 'empty', meaning it doesn't have anything
/// worth persisting in it. Specifically:
///
/// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None
/// - `class_name` is either None or not Some("Folder")
/// - `name` is None
#[inline]
pub fn is_empty(&self) -> bool {
self.attributes.is_empty()
&& self.properties.is_empty()
&& self.ignore_unknown_instances.is_none()
&& self.name.is_none()
&& if let Some(class) = &self.class_name {
class == "Folder"
} else {

View File

@@ -61,10 +61,6 @@ pub use self::{
/// This will inspect the path and find the appropriate middleware for it,
/// taking user-written rules into account. Then, it will attempt to convert
/// the path into an InstanceSnapshot using that middleware.
///
/// If a git filter is active in the context and the path is not acknowledged
/// (i.e., the file hasn't changed since the base git reference), this function
/// returns `Ok(None)` to skip syncing that file.
#[profiling::function]
pub fn snapshot_from_vfs(
context: &InstanceContext,
@@ -76,16 +72,6 @@ pub fn snapshot_from_vfs(
None => return Ok(None),
};
// Check if this path is acknowledged by the git filter.
// If not, skip this path entirely.
if !context.is_path_acknowledged(path) {
log::trace!(
"Skipping path {} (not acknowledged by git filter)",
path.display()
);
return Ok(None);
}
if meta.is_dir() {
let (middleware, dir_name, init_path) = get_dir_middleware(vfs, path)?;
// TODO: Support user defined init paths
@@ -105,7 +91,9 @@ pub fn snapshot_from_vfs(
// TODO: Is this even necessary anymore?
match file_name {
"init.server.luau" | "init.server.lua" | "init.client.luau" | "init.client.lua"
| "init.luau" | "init.lua" | "init.csv" => return Ok(None),
| "init.plugin.luau" | "init.plugin.lua" | "init.luau" | "init.lua" | "init.csv" => {
return Ok(None)
}
_ => {}
}
@@ -138,6 +126,8 @@ fn get_dir_middleware<'path>(
(Middleware::ServerScriptDir, "init.server.lua"),
(Middleware::ClientScriptDir, "init.client.luau"),
(Middleware::ClientScriptDir, "init.client.lua"),
(Middleware::PluginScriptDir, "init.plugin.lua"),
(Middleware::PluginScriptDir, "init.plugin.luau"),
(Middleware::CsvDir, "init.csv"),
]
});
@@ -219,6 +209,8 @@ pub enum Middleware {
#[serde(skip_deserializing)]
ClientScriptDir,
#[serde(skip_deserializing)]
PluginScriptDir,
#[serde(skip_deserializing)]
ModuleScriptDir,
#[serde(skip_deserializing)]
CsvDir,
@@ -227,10 +219,6 @@ pub enum Middleware {
impl Middleware {
/// Creates a snapshot for the given path from the Middleware with
/// the provided name.
///
/// When a git filter is active in the context, `ignore_unknown_instances`
/// will be set to `true` on all generated snapshots to preserve descendants
/// in Studio that are not tracked by Rojo.
fn snapshot(
&self,
context: &InstanceContext,
@@ -273,6 +261,9 @@ impl Middleware {
Self::ClientScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Client)
}
Self::PluginScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Plugin)
}
Self::ModuleScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Module)
}
@@ -280,14 +271,6 @@ impl Middleware {
};
if let Ok(Some(ref mut snapshot)) = output {
snapshot.metadata.middleware = Some(*self);
// When git filter is active, force ignore_unknown_instances to true
// so that we don't delete children in Studio that aren't tracked.
if context.has_git_filter() {
snapshot.metadata.ignore_unknown_instances = true;
// Also apply this recursively to all children
set_ignore_unknown_instances_recursive(&mut snapshot.children);
}
}
output
}
@@ -323,6 +306,7 @@ impl Middleware {
Middleware::Dir => syncback_dir(snapshot),
Middleware::ServerScriptDir => syncback_lua_init(ScriptType::Server, snapshot),
Middleware::ClientScriptDir => syncback_lua_init(ScriptType::Client, snapshot),
Middleware::PluginScriptDir => syncback_lua_init(ScriptType::Plugin, snapshot),
Middleware::ModuleScriptDir => syncback_lua_init(ScriptType::Module, snapshot),
Middleware::CsvDir => syncback_csv_init(snapshot),
@@ -344,6 +328,7 @@ impl Middleware {
Middleware::Dir
| Middleware::ServerScriptDir
| Middleware::ClientScriptDir
| Middleware::PluginScriptDir
| Middleware::ModuleScriptDir
| Middleware::CsvDir
)
@@ -391,16 +376,6 @@ impl Middleware {
}
}
/// Recursively sets `ignore_unknown_instances` to `true` on all children.
/// This is used when git filter is active to ensure we don't delete
/// children in Studio that aren't tracked by Rojo.
fn set_ignore_unknown_instances_recursive(children: &mut [InstanceSnapshot]) {
for child in children {
child.metadata.ignore_unknown_instances = true;
set_ignore_unknown_instances_recursive(&mut child.children);
}
}
/// A helper for easily defining a SyncRule. Arguments are passed literally
/// to this macro in the order `include`, `middleware`, `suffix`,
/// and `exclude`. Both `suffix` and `exclude` are optional.

View File

@@ -83,19 +83,6 @@ pub fn snapshot_project(
// file being updated.
snapshot.metadata.relevant_paths.push(path.to_path_buf());
// When git filter is active, also register the project folder as a
// relevant path. This serves as a catch-all so that file changes
// not under any specific $path node can still walk up the directory
// tree and trigger a re-snapshot of the entire project.
if context.has_git_filter() {
if let Some(folder) = path.parent() {
let normalized = vfs
.canonicalize(folder)
.unwrap_or_else(|_| folder.to_path_buf());
snapshot.metadata.relevant_paths.push(normalized);
}
}
Ok(Some(snapshot))
}
None => Ok(None),
@@ -150,26 +137,6 @@ pub fn snapshot_project_node(
// Take the snapshot's metadata as-is, which will be mutated later
// on.
metadata = snapshot.metadata;
} else if context.has_git_filter() {
// When the git filter is active and the $path was filtered out
// (no acknowledged files yet), we still need to register the path
// in relevant_paths. This allows the change processor to map file
// changes in this directory back to this project node instance,
// triggering a re-snapshot that will pick up newly modified files.
let normalized = vfs
.canonicalize(full_path.as_ref())
.unwrap_or_else(|_| full_path.to_path_buf());
metadata.relevant_paths.push(normalized);
// The VFS only sets up file watches via read() and read_dir(),
// not via metadata(). Since the git filter caused snapshot_from_vfs
// to return early (before read_dir was called), the VFS is not
// watching this path. We must read the directory here to ensure
// the VFS sets up a recursive watch, otherwise file change events
// will never fire and live sync won't detect modifications.
if full_path.is_dir() {
let _ = vfs.read_dir(&full_path);
}
}
}
@@ -225,17 +192,6 @@ pub fn snapshot_project_node(
}
(_, None, _, Some(PathNode::Required(path))) => {
// If git filter is active and the path was filtered out, treat it
// as if the path was optional and skip this node.
if context.has_git_filter() {
log::trace!(
"Skipping project node '{}' because its path was filtered by git filter: {}",
instance_name,
path.display()
);
return Ok(None);
}
anyhow::bail!(
"Rojo project referred to a file using $path that could not be turned into a Roblox Instance by Rojo.\n\
Check that the file exists and is a file type known by Rojo.\n\
@@ -326,12 +282,7 @@ pub fn snapshot_project_node(
// If the user didn't specify it AND $path was not specified (meaning
// there's no existing value we'd be stepping on from a project file or meta
// file), set it to true.
//
// When git filter is active, always set to true to preserve descendants
// in Studio that are not tracked by Rojo.
if context.has_git_filter() {
metadata.ignore_unknown_instances = true;
} else if let Some(ignore) = node.ignore_unknown_instances {
if let Some(ignore) = node.ignore_unknown_instances {
metadata.ignore_unknown_instances = ignore;
} else if node.path.is_none() {
// TODO: Introduce a strict mode where $ignoreUnknownInstances is never
@@ -393,6 +344,11 @@ pub fn syncback_project<'sync>(
let mut new_child_map = HashMap::new();
let mut node_changed_map = Vec::new();
// Tracks whether any stale default-valued properties were removed from
// project nodes. If so, we must reserialize even if
// project_node_should_reserialize wouldn't otherwise detect a change
// (it only compares node properties forward, not in reverse).
let mut removed_stale_properties = false;
let mut node_queue = VecDeque::with_capacity(1);
node_queue.push_back((&mut project.tree, old_inst, snapshot.new_inst()));
@@ -451,10 +407,12 @@ pub fn syncback_project<'sync>(
// We only want to set properties if it needs it.
if !middleware.handles_own_properties() {
project_node_property_syncback_path(snapshot, new_inst, node);
removed_stale_properties |=
project_node_property_syncback_path(snapshot, new_inst, node);
}
} else {
project_node_property_syncback_no_path(snapshot, new_inst, node);
removed_stale_properties |=
project_node_property_syncback_no_path(snapshot, new_inst, node);
}
for child_ref in new_inst.children() {
@@ -556,12 +514,18 @@ pub fn syncback_project<'sync>(
}
let mut fs_snapshot = FsSnapshot::new();
for (node_properties, node_attributes, old_inst) in node_changed_map {
if project_node_should_reserialize(node_properties, node_attributes, old_inst)? {
fs_snapshot.add_file(project_path, serde_json::to_vec_pretty(&project)?);
break;
let mut needs_reserialize = removed_stale_properties;
if !needs_reserialize {
for (node_properties, node_attributes, old_inst) in node_changed_map {
if project_node_should_reserialize(node_properties, node_attributes, old_inst)? {
needs_reserialize = true;
break;
}
}
}
if needs_reserialize {
fs_snapshot.add_file(project_path, serde_json::to_vec_pretty(&project)?);
}
Ok(SyncbackReturn {
fs_snapshot,
@@ -570,15 +534,18 @@ pub fn syncback_project<'sync>(
})
}
/// Syncs properties from the new instance into the project node.
/// Returns `true` if any stale properties were removed (i.e. properties
/// that existed in the project node but are now at their engine default).
fn project_node_property_syncback(
_snapshot: &SyncbackSnapshot,
filtered_properties: UstrMap<&Variant>,
new_inst: &Instance,
node: &mut ProjectNode,
) {
) -> bool {
let properties = &mut node.properties;
let mut attributes = BTreeMap::new();
for (name, value) in filtered_properties {
for (&name, &value) in &filtered_properties {
match value {
Variant::Attributes(attrs) => {
for (attr_name, attr_value) in attrs.iter() {
@@ -601,14 +568,48 @@ fn project_node_property_syncback(
}
}
}
// Remove stale properties: entries that exist in the project node's
// $properties but are no longer in the filtered (non-default) properties
// from the instance. This handles the case where Studio resets a property
// to its engine default — filter_properties won't include it, so we need
// to clean up the now-stale project entry.
let class_data = rbx_reflection_database::get()
.ok()
.and_then(|db| db.classes.get(new_inst.class.as_str()));
let len_before = properties.len();
properties.retain(|prop_name, _| {
if filtered_properties.contains_key(prop_name) {
return true;
}
// Only remove if the property has a known default value in the
// reflection database. If there's no default, the property might be
// absent from the instance for other reasons (e.g. unknown property),
// so we conservatively keep it.
if let Some(data) = &class_data {
if data.default_properties.contains_key(prop_name.as_str()) {
log::debug!(
"Removing stale property '{}' from project node for class '{}': \
value has been reset to engine default",
prop_name,
new_inst.class
);
return false;
}
}
true
});
let removed_stale = properties.len() < len_before;
node.attributes = attributes;
removed_stale
}
fn project_node_property_syncback_path(
snapshot: &SyncbackSnapshot,
new_inst: &Instance,
node: &mut ProjectNode,
) {
) -> bool {
let filtered_properties = snapshot
.get_path_filtered_properties(new_inst.referent())
.unwrap();
@@ -619,7 +620,7 @@ fn project_node_property_syncback_no_path(
snapshot: &SyncbackSnapshot,
new_inst: &Instance,
node: &mut ProjectNode,
) {
) -> bool {
let filtered_properties = filter_properties(snapshot.project(), new_inst);
project_node_property_syncback(snapshot, filtered_properties, new_inst, node)
}

View File

@@ -15,6 +15,8 @@ metadata:
- /root/init.server.luau
- /root/init.client.lua
- /root/init.client.luau
- /root/init.plugin.lua
- /root/init.plugin.luau
- /root/init.csv
- /root/init.meta.json
- /root/init.meta.jsonc

View File

@@ -15,6 +15,8 @@ metadata:
- /root/init.server.luau
- /root/init.client.lua
- /root/init.client.luau
- /root/init.plugin.lua
- /root/init.plugin.luau
- /root/init.csv
- /root/init.meta.json
- /root/init.meta.jsonc

View File

@@ -15,6 +15,8 @@ metadata:
- /foo/init.server.luau
- /foo/init.client.lua
- /foo/init.client.luau
- /foo/init.plugin.lua
- /foo/init.plugin.luau
- /foo/init.csv
- /foo/init.meta.json
- /foo/init.meta.jsonc

View File

@@ -15,6 +15,8 @@ metadata:
- /foo/init.server.luau
- /foo/init.client.lua
- /foo/init.client.luau
- /foo/init.plugin.lua
- /foo/init.plugin.luau
- /foo/init.csv
- /foo/init.meta.json
- /foo/init.meta.jsonc
@@ -40,6 +42,8 @@ children:
- /foo/Child/init.server.luau
- /foo/Child/init.client.lua
- /foo/Child/init.client.luau
- /foo/Child/init.plugin.lua
- /foo/Child/init.plugin.luau
- /foo/Child/init.csv
- /foo/Child/init.meta.json
- /foo/Child/init.meta.jsonc

View File

@@ -15,6 +15,8 @@ metadata:
- /root/init.server.luau
- /root/init.client.lua
- /root/init.client.luau
- /root/init.plugin.lua
- /root/init.plugin.luau
- /root/init.csv
- /root/init.meta.json
- /root/init.meta.jsonc

View File

@@ -15,6 +15,8 @@ metadata:
- /root/init.server.luau
- /root/init.client.lua
- /root/init.client.luau
- /root/init.plugin.lua
- /root/init.plugin.luau
- /root/init.csv
- /root/init.meta.json
- /root/init.meta.jsonc

View File

@@ -58,17 +58,8 @@ pub fn syncback_txt<'sync>(
if !meta.is_empty() {
let parent = snapshot.path.parent_err()?;
let meta_stem = snapshot.middleware
.and_then(|mw| {
let ext = format!(".{}", crate::syncback::extension_for_middleware(mw));
snapshot.path.file_name()
.and_then(|n| n.to_str())
.and_then(|s| s.strip_suffix(ext.as_str()))
.map(str::to_owned)
})
.unwrap_or_else(|| new_inst.name.clone());
fs_snapshot.add_file(
parent.join(format!("{meta_stem}.meta.json")),
parent.join(format!("{}.meta.json", new_inst.name)),
serde_json::to_vec_pretty(&meta).context("could not serialize metadata")?,
);
}

View File

@@ -8,11 +8,11 @@ use rbx_dom_weak::Instance;
use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware};
pub fn name_for_inst<'a>(
pub fn name_for_inst<'old>(
middleware: Middleware,
new_inst: &'a Instance,
old_inst: Option<InstanceWithMeta<'a>>,
) -> anyhow::Result<Cow<'a, str>> {
new_inst: &Instance,
old_inst: Option<InstanceWithMeta<'old>>,
) -> anyhow::Result<Cow<'old, str>> {
if let Some(old_inst) = old_inst {
if let Some(source) = old_inst.metadata().relevant_paths.first() {
source
@@ -35,34 +35,15 @@ pub fn name_for_inst<'a>(
| Middleware::CsvDir
| Middleware::ServerScriptDir
| Middleware::ClientScriptDir
| Middleware::ModuleScriptDir => {
let name = if validate_file_name(&new_inst.name).is_err() {
Cow::Owned(slugify_name(&new_inst.name))
} else {
Cow::Borrowed(new_inst.name.as_str())
};
// Prefix "init" to avoid colliding with reserved init files.
if name.to_lowercase() == "init" {
Cow::Owned(format!("_{name}"))
} else {
name
}
}
| Middleware::PluginScriptDir
| Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()),
_ => {
let extension = extension_for_middleware(middleware);
let slugified;
let stem: &str = if validate_file_name(&new_inst.name).is_err() {
slugified = slugify_name(&new_inst.name);
&slugified
} else {
&new_inst.name
};
// Prefix "init" stems to avoid colliding with reserved init files.
if stem.to_lowercase() == "init" {
Cow::Owned(format!("_{stem}.{extension}"))
} else {
Cow::Owned(format!("{stem}.{extension}"))
}
let name = &new_inst.name;
validate_file_name(name).with_context(|| {
format!("name '{name}' is not legal to write to the file system")
})?;
Cow::Owned(format!("{name}.{extension}"))
}
})
}
@@ -98,6 +79,7 @@ pub fn extension_for_middleware(middleware: Middleware) -> &'static str {
| Middleware::CsvDir
| Middleware::ServerScriptDir
| Middleware::ClientScriptDir
| Middleware::PluginScriptDir
| Middleware::ModuleScriptDir => {
unimplemented!("directory middleware requires special treatment")
}
@@ -114,39 +96,6 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [
/// in a file's name.
const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\'];
/// Slugifies a name by replacing forbidden characters with underscores
/// and ensuring the result is a valid file name
pub fn slugify_name(name: &str) -> String {
let mut result = String::with_capacity(name.len());
for ch in name.chars() {
if FORBIDDEN_CHARS.contains(&ch) {
result.push('_');
} else {
result.push(ch);
}
}
// Handle Windows reserved names by appending an underscore
let result_lower = result.to_lowercase();
for forbidden in INVALID_WINDOWS_NAMES {
if result_lower == forbidden.to_lowercase() {
result.push('_');
break;
}
}
while result.ends_with(' ') || result.ends_with('.') {
result.pop();
}
if result.is_empty() || result.chars().all(|c| c == '_') {
result = "instance".to_string();
}
result
}
/// Validates a provided file name to ensure it's allowed on the file system. An
/// error is returned if the name isn't allowed, indicating why.
/// This takes into account rules for Windows, MacOS, and Linux.

View File

@@ -21,14 +21,14 @@ use std::{
};
use crate::{
glob::Glob,
glob::{Glob, IgnorableGlob},
snapshot::{InstanceWithMeta, RojoTree},
snapshot_middleware::Middleware,
syncback::ref_properties::{collect_referents, link_referents},
Project,
};
pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, validate_file_name};
pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name};
pub use fs_snapshot::FsSnapshot;
pub use hash::*;
pub use property_filter::{filter_properties, filter_properties_preallocated};
@@ -52,7 +52,6 @@ pub fn syncback_loop(
old_tree: &mut RojoTree,
mut new_tree: WeakDom,
project: &Project,
force_json: bool,
) -> anyhow::Result<FsSnapshot> {
let ignore_patterns = project
.syncback_rules
@@ -154,7 +153,6 @@ pub fn syncback_loop(
old_tree,
new_tree: &new_tree,
project,
force_json,
};
let mut snapshots = vec![SyncbackSnapshot {
@@ -199,7 +197,7 @@ pub fn syncback_loop(
}
}
let middleware = get_best_middleware(&snapshot, force_json);
let middleware = get_best_middleware(&snapshot);
log::trace!(
"Middleware for {inst_path} is {:?} (path is {})",
@@ -215,14 +213,10 @@ pub fn syncback_loop(
let syncback = match middleware.syncback(&snapshot) {
Ok(syncback) => syncback,
Err(err) if middleware == Middleware::Dir => {
let new_middleware = if force_json {
Middleware::JsonModel
} else {
match env::var(DEBUG_MODEL_FORMAT_VAR) {
Ok(value) if value == "1" => Middleware::Rbxmx,
Ok(value) if value == "2" => Middleware::JsonModel,
_ => Middleware::Rbxm,
}
let new_middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
Ok(value) if value == "1" => Middleware::Rbxmx,
Ok(value) if value == "2" => Middleware::JsonModel,
_ => Middleware::Rbxm,
};
let file_name = snapshot
.path
@@ -301,7 +295,7 @@ pub struct SyncbackReturn<'sync> {
pub removed_children: Vec<InstanceWithMeta<'sync>>,
}
pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Middleware {
pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
// At some point, we're better off using an O(1) method for checking
// equality for classes like this.
static JSON_MODEL_CLASSES: OnceLock<HashSet<&str>> = OnceLock::new();
@@ -365,6 +359,7 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Mid
middleware = match middleware {
Middleware::ServerScript => Middleware::ServerScriptDir,
Middleware::ClientScript => Middleware::ClientScriptDir,
Middleware::PluginScript => Middleware::PluginScriptDir,
Middleware::ModuleScript => Middleware::ModuleScriptDir,
Middleware::Csv => Middleware::CsvDir,
Middleware::JsonModel | Middleware::Text => Middleware::Dir,
@@ -373,18 +368,10 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Mid
}
if middleware == Middleware::Rbxm {
middleware = if force_json {
if !inst.children().is_empty() {
Middleware::Dir
} else {
Middleware::JsonModel
}
} else {
match env::var(DEBUG_MODEL_FORMAT_VAR) {
Ok(value) if value == "1" => Middleware::Rbxmx,
Ok(value) if value == "2" => Middleware::JsonModel,
_ => Middleware::Rbxm,
}
middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
Ok(value) if value == "1" => Middleware::Rbxmx,
Ok(value) if value == "2" => Middleware::JsonModel,
_ => Middleware::Rbxm,
}
}
@@ -428,18 +415,18 @@ pub struct SyncbackRules {
}
impl SyncbackRules {
pub fn compile_globs(&self) -> anyhow::Result<Vec<Glob>> {
pub fn compile_globs(&self) -> anyhow::Result<Vec<IgnorableGlob>> {
let mut globs = Vec::with_capacity(self.ignore_paths.len());
let dir_ignore_paths = self.create_ignore_dir_paths.unwrap_or(true);
for pattern in &self.ignore_paths {
let glob = Glob::new(pattern)
let glob = IgnorableGlob::new(pattern)
.with_context(|| format!("the pattern '{pattern}' is not a valid glob"))?;
globs.push(glob);
if dir_ignore_paths {
if let Some(dir_pattern) = pattern.strip_suffix("/**") {
if let Ok(glob) = Glob::new(dir_pattern) {
if let Ok(glob) = IgnorableGlob::new(dir_pattern) {
globs.push(glob)
}
}
@@ -450,7 +437,7 @@ impl SyncbackRules {
}
}
fn is_valid_path(globs: &Option<Vec<Glob>>, base_path: &Path, path: &Path) -> bool {
fn is_valid_path(globs: &Option<Vec<IgnorableGlob>>, base_path: &Path, path: &Path) -> bool {
let git_glob = GIT_IGNORE_GLOB.get_or_init(|| Glob::new(".git/**").unwrap());
let test_path = match path.strip_prefix(base_path) {
Ok(suffix) => suffix,
@@ -460,11 +447,16 @@ fn is_valid_path(globs: &Option<Vec<Glob>>, base_path: &Path, path: &Path) -> bo
return false;
}
if let Some(ref ignore_paths) = globs {
// Gitignore-style "last match wins"
let mut ignored = false;
for glob in ignore_paths {
if glob.is_match(test_path) {
return false;
ignored = !glob.is_negation();
}
}
if ignored {
return false;
}
}
true
}
@@ -552,3 +544,71 @@ fn strip_unknown_root_children(new: &mut WeakDom, old: &RojoTree) {
new.destroy(child_ref);
}
}
#[cfg(test)]
mod test {
use super::*;
fn rules(ignore_paths: &[&str], create_ignore_dir_paths: Option<bool>) -> SyncbackRules {
SyncbackRules {
ignore_trees: Vec::new(),
ignore_paths: ignore_paths.iter().map(|s| s.to_string()).collect(),
ignore_properties: IndexMap::new(),
sync_current_camera: None,
sync_unscriptable: None,
ignore_referents: None,
create_ignore_dir_paths,
}
}
#[test]
fn ignore_paths_negation() {
let globs = Some(
rules(&["**/*.lua", "!keep.lua"], Some(false))
.compile_globs()
.unwrap(),
);
let base = Path::new("/test");
// A later negation re-includes a path matched by an earlier pattern.
assert!(!is_valid_path(&globs, base, Path::new("/test/foo.lua")));
assert!(is_valid_path(&globs, base, Path::new("/test/keep.lua")));
// Paths matched by no rule are valid.
assert!(is_valid_path(&globs, base, Path::new("/test/plain.txt")));
}
#[test]
fn ignore_paths_negation_with_dir_expansion() {
// With `create_ignore_dir_paths`, a negated `foo/**` pattern should also
// re-include the `foo` directory itself, mirroring the file rule.
let globs = Some(
rules(&["**/*", "!keep/**"], Some(true))
.compile_globs()
.unwrap(),
);
let base = Path::new("/test");
assert!(!is_valid_path(&globs, base, Path::new("/test/drop/a.lua")));
assert!(is_valid_path(&globs, base, Path::new("/test/keep")));
assert!(is_valid_path(&globs, base, Path::new("/test/keep/a.lua")));
}
#[test]
fn ignore_paths_escaped_bang_is_literal() {
// `\!literal.lua` should ignore a file literally named `!literal.lua`
// rather than being parsed as a negation.
let globs = Some(
rules(&[r"\!literal.lua"], Some(false))
.compile_globs()
.unwrap(),
);
let base = Path::new("/test");
assert!(!globs.as_ref().unwrap()[0].is_negation());
assert!(!is_valid_path(
&globs,
base,
Path::new("/test/!literal.lua")
));
}
}

View File

@@ -20,7 +20,6 @@ pub struct SyncbackData<'sync> {
pub(super) old_tree: &'sync RojoTree,
pub(super) new_tree: &'sync WeakDom,
pub(super) project: &'sync Project,
pub(super) force_json: bool,
}
pub struct SyncbackSnapshot<'sync> {
@@ -44,7 +43,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
path: PathBuf::new(),
middleware: None,
};
let middleware = get_best_middleware(&snapshot, self.data.force_json);
let middleware = get_best_middleware(&snapshot);
let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
snapshot.path = self.path.join(name.as_ref());
@@ -70,7 +69,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
path: PathBuf::new(),
middleware: None,
};
let middleware = get_best_middleware(&snapshot, self.data.force_json);
let middleware = get_best_middleware(&snapshot);
let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
snapshot.path = base_path.join(name.as_ref());
@@ -238,24 +237,6 @@ pub fn inst_path(dom: &WeakDom, referent: Ref) -> String {
path.join("/")
}
impl<'sync> SyncbackData<'sync> {
/// Constructs a `SyncbackData` for use in unit tests.
#[cfg(test)]
pub fn for_test(
vfs: &'sync Vfs,
old_tree: &'sync RojoTree,
new_tree: &'sync WeakDom,
project: &'sync Project,
) -> Self {
Self {
vfs,
old_tree,
new_tree,
project,
}
}
}
#[cfg(test)]
mod test {
use rbx_dom_weak::{InstanceBuilder, WeakDom};

View File

@@ -1,7 +1,7 @@
//! Defines Rojo's HTTP API, all under /api. These endpoints generally return
//! JSON.
use std::{collections::HashMap, fs, path::PathBuf, str::FromStr, sync::Arc};
use std::{collections::HashMap, fs, net::SocketAddr, path::PathBuf, str::FromStr, sync::Arc};
use futures::{sink::SinkExt, stream::StreamExt};
use hyper::{body, Body, Method, Request, Response, StatusCode};
@@ -21,6 +21,7 @@ use crate::{
ServerInfoResponse, SocketPacket, SocketPacketBody, SocketPacketType, SubscribeMessage,
WriteRequest, WriteResponse, PROTOCOL_VERSION, SERVER_VERSION,
},
origin::canonical,
util::{deserialize_msgpack, msgpack, msgpack_ok, serialize_msgpack},
},
web_api::{
@@ -28,8 +29,12 @@ use crate::{
},
};
pub async fn call(serve_session: Arc<ServeSession>, mut request: Request<Body>) -> Response<Body> {
let service = ApiService::new(serve_session);
pub async fn call(
serve_session: Arc<ServeSession>,
remote_addr: SocketAddr,
mut request: Request<Body>,
) -> Response<Body> {
let service = ApiService::new(serve_session, remote_addr);
match (request.method(), request.uri().path()) {
(&Method::GET, "/api/rojo") => service.handle_api_rojo().await,
@@ -65,11 +70,15 @@ pub async fn call(serve_session: Arc<ServeSession>, mut request: Request<Body>)
pub struct ApiService {
serve_session: Arc<ServeSession>,
remote_addr: SocketAddr,
}
impl ApiService {
pub fn new(serve_session: Arc<ServeSession>) -> Self {
ApiService { serve_session }
pub fn new(serve_session: Arc<ServeSession>, remote_addr: SocketAddr) -> Self {
ApiService {
serve_session,
remote_addr,
}
}
/// Get a summary of information about the server
@@ -267,7 +276,7 @@ impl ApiService {
response_dom.transfer_within(child_ref, object_value);
} else {
msgpack(
return msgpack(
ErrorResponse::bad_request(format!("provided id {id} is not in the tree")),
StatusCode::BAD_REQUEST,
);
@@ -348,6 +357,30 @@ impl ApiService {
/// Open a script with the given ID in the user's default text editor.
async fn handle_api_open(&self, request: Request<Body>) -> Response<Body> {
// Opening a file launches a local program, so it must never be reachable
// by a remote client even when the server is bound to an exposed address.
//
// `remote_addr` is the immediate peer, which is the best locality signal
// we have: the legitimate caller is a sandboxed Roblox plugin whose only
// credential is being able to reach the port, so there is no secret to
// authenticate it with. A connection forwarded over loopback by an
// SSH/Tailscale tunnel or a local reverse proxy therefore appears local
// and is allowed. That is delegated trust rather than a bypass: by
// standing up that tunnel or proxy the user has decided the remote end is
// trusted, and reachability is bounded by that hop's own authentication
// (e.g. SSH keys or Tailscale ACLs). This gate only stops direct,
// unauthenticated peers.
//
// An IPv4 client reaching a dual-stack (`::`) bind appears as an
// IPv4-mapped IPv6 peer (`::ffff:127.0.0.1`), so canonicalize to the bare
// IPv4 form before the loopback test, matching `origin`'s handling.
if !canonical(self.remote_addr.ip()).is_loopback() {
return msgpack(
ErrorResponse::forbidden("/api/open is only available to local clients"),
StatusCode::FORBIDDEN,
);
}
let argument = &request.uri().path()["/api/open/".len()..];
let requested_id = match Ref::from_str(argument) {
Ok(id) => id,

View File

@@ -290,6 +290,13 @@ impl ErrorResponse {
}
}
pub fn forbidden<S: Into<String>>(details: S) -> Self {
Self {
kind: ErrorResponseKind::Forbidden,
details: details.into(),
}
}
pub fn internal_error<S: Into<String>>(details: S) -> Self {
Self {
kind: ErrorResponseKind::InternalError,
@@ -302,5 +309,6 @@ impl ErrorResponse {
pub enum ErrorResponseKind {
NotFound,
BadRequest,
Forbidden,
InternalError,
}

View File

@@ -5,6 +5,7 @@
mod api;
mod assets;
pub mod interface;
mod origin;
mod ui;
mod util;
@@ -12,8 +13,9 @@ use std::convert::Infallible;
use std::net::SocketAddr;
use std::sync::Arc;
use anyhow::Context;
use hyper::{
server::Server,
server::{conn::AddrStream, Server},
service::{make_service_fn, service_fn},
Body, Request,
};
@@ -30,19 +32,44 @@ impl LiveServer {
LiveServer { serve_session }
}
pub fn start(self, address: SocketAddr) {
/// Starts the server on the given address, blocking until it stops.
///
/// `allowed_hosts` are extra `Host`/`Origin` values to accept in addition to
/// localhost and the bind address (see [`origin::allowed_hosts`]).
///
/// `on_listening` is invoked once the server has successfully bound to the
/// address, so callers can defer printing any "listening" message until
/// after binding can no longer fail (e.g. due to the port being in use).
pub fn start(
self,
address: SocketAddr,
allowed_hosts: Vec<String>,
on_listening: impl FnOnce(),
) -> anyhow::Result<()> {
let serve_session = Arc::clone(&self.serve_session);
let allowed_hosts = origin::allowed_hosts(address.ip(), address.port(), &allowed_hosts);
let make_service = make_service_fn(move |_conn| {
let make_service = make_service_fn(move |conn: &AddrStream| {
let serve_session = Arc::clone(&serve_session);
let allowed_hosts = allowed_hosts.clone();
let remote_addr = conn.remote_addr();
async {
async move {
let service = move |req: Request<Body>| {
let serve_session = Arc::clone(&serve_session);
let allowed_hosts = allowed_hosts.clone();
async move {
// Reject cross-origin requests before doing any work, to
// defend the local server against DNS rebinding.
if let Some(response) =
origin::check_request_origin(&req, allowed_hosts.as_ref())
{
return Ok::<_, Infallible>(response);
}
if req.uri().path().starts_with("/api") {
Ok::<_, Infallible>(api::call(serve_session, req).await)
Ok::<_, Infallible>(api::call(serve_session, remote_addr, req).await)
} else {
Ok::<_, Infallible>(ui::call(serve_session, req).await)
}
@@ -53,9 +80,25 @@ impl LiveServer {
}
});
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for the web server")?;
let _guard = rt.enter();
let server = Server::bind(&address).serve(make_service);
rt.block_on(server).unwrap();
let server = Server::try_bind(&address)
.with_context(|| {
format!(
"Could not start the Rojo server on {address}.\n\
The address may already be in use or reserved. Another Rojo server might already \
be running, or another program may be using that port.\n\
You can pick a different port with the --port option."
)
})?
.serve(make_service);
// Binding succeeded, so it's now safe to tell the user we're listening.
on_listening();
rt.block_on(server)
.context("The Rojo web server encountered a fatal error")?;
Ok(())
}
}

589
src/web/origin.rs Normal file
View File

@@ -0,0 +1,589 @@
//! Host/Origin validation used to defend the local Rojo server against DNS
//! rebinding attacks.
//!
//! When Rojo is bound to a loopback address (the default), a malicious web page
//! the developer visits cannot read the API responses directly because of the
//! browser Same-Origin Policy. However, a DNS rebinding attack can defeat that:
//! the page points its own hostname at `127.0.0.1` after loading, so the browser
//! treats requests to the Rojo server as same-origin. Validating that the `Host`
//! (and, if present, `Origin`) header refers to an address we recognize blocks
//! this, because the rebound request still carries the attacker's hostname, which
//! is a domain name rather than one of the IP literals we accept.
//!
//! Enforcement covers two kinds of bind:
//!
//! * Loopback (the default): only `localhost` and loopback literals are
//! accepted.
//! * A specific private/LAN address: `localhost`, loopback literals, and that exact bind
//! IP are accepted. Because the defense works by rejecting any `Host` that
//! isn't a recognized IP literal, clients must connect to a private bind by
//! IP. A hostname (e.g. `mypc.local`) is indistinguishable from an attacker
//! domain and is rejected.
//!
//! Enforcement is disabled for unspecified (`0.0.0.0`/`::`) and public binds: the
//! user has asked for broad, possibly-public exposure where arbitrary hostnames
//! may legitimately resolve to the server, so we can't build a meaningful
//! allowlist. Those binds get a startup warning instead. Two consequences worth
//! being explicit about: such a bind has no rebinding protection even for a
//! browser on the same machine; and even on a protected private bind this check
//! does nothing against a hostile peer already on the LAN, who can reach the
//! unauthenticated API directly. Both are the network-exposure risk the startup
//! warning addresses, not something the `Host` check is meant to cover.
//!
//! The allowlist can be widened with extra hosts (the `--allowed-hosts` CLI
//! option or a project's `serveAllowedHosts`), for example a hostname like
//! `mypc.lan` for reaching a network-exposed server by name. Listing any extra
//! host also turns enforcement back on for an unspecified or public bind that
//! would otherwise disable it, restricting that bind to localhost, the bind IP,
//! and the listed hosts.
use std::net::IpAddr;
use hyper::{
header::{HOST, ORIGIN},
http::uri::{Authority, Uri},
Body, Request, Response, StatusCode,
};
use crate::web::util::response;
/// The set of `Host`/`Origin` values accepted while enforcement is active.
#[derive(Debug, Clone)]
pub struct AllowedHosts {
port: u16,
/// The bind IP accepted in addition to `localhost`/loopback, set only for a
/// private (LAN) bind or a specific public bind that has extra hosts. `None`
/// for a loopback or unspecified bind, which has no single address to add.
bind_ip: Option<IpAddr>,
/// Extra `Host`/`Origin` values the user explicitly allowed, via the
/// `--allowed-hosts` option or a project's `serveAllowedHosts`. These are
/// hostnames such as `mypc.lan`, or IP literals, accepted in addition to
/// localhost and the bind IP. Each entry is already passed through
/// [`normalize_host`].
extra_hosts: Vec<String>,
}
impl AllowedHosts {
/// Returns whether the given host and optional port are allowed. The host is
/// accepted if it is `localhost`, a loopback IP literal, (on a private or
/// specific public bind) the exact bind IP, or one of the explicitly allowed
/// extra hosts. A request with no explicit port is accepted (the host
/// already has to be one we recognize).
fn allows(&self, host: &str, port: Option<u16>) -> bool {
let host = normalize_host(host);
let host_ok = host.eq_ignore_ascii_case("localhost")
|| host
.parse::<IpAddr>()
.ok()
.map(canonical)
.is_some_and(|ip| ip.is_loopback() || self.bind_ip == Some(ip))
|| self.allows_extra(host);
host_ok && port.is_none_or(|port| port == self.port)
}
/// Returns whether `host` matches one of the explicitly allowed extra hosts.
/// Entries that are IP literals are compared as addresses, so equivalent
/// forms (such as an IPv4-mapped IPv6 literal) match; everything else is
/// compared as a case-insensitive hostname.
fn allows_extra(&self, host: &str) -> bool {
let host_ip = host.parse::<IpAddr>().ok().map(canonical);
self.extra_hosts.iter().any(|allowed| {
match (host_ip, allowed.parse::<IpAddr>().map(canonical)) {
(Some(host_ip), Ok(allowed_ip)) => host_ip == allowed_ip,
_ => host.eq_ignore_ascii_case(allowed),
}
})
}
}
/// Builds the allowlist for a given bind address and any extra allowed hosts.
/// Returns `None` (validation disabled) when bound to an unspecified
/// (`0.0.0.0`/`::`) or public address and no extra hosts were given, where
/// arbitrary hostnames may legitimately resolve to the server. Listing extra
/// hosts keeps validation on even for those binds.
pub fn allowed_hosts(bind: IpAddr, port: u16, extra: &[String]) -> Option<AllowedHosts> {
let extra_hosts: Vec<String> = extra
.iter()
.map(|host| normalize_host(host.trim()).to_owned())
.filter(|host| !host.is_empty())
.collect();
if bind.is_loopback() {
Some(AllowedHosts {
port,
bind_ip: None,
extra_hosts,
})
} else if is_private_bind(bind) {
Some(AllowedHosts {
port,
bind_ip: Some(canonical(bind)),
extra_hosts,
})
} else if !extra_hosts.is_empty() {
// The bind is unspecified or public, where validation is normally
// disabled. By listing explicit hosts the user has opted back into it,
// so we accept localhost, the bind IP (when it is a specific address),
// and those hosts. An unspecified bind (`0.0.0.0`/`::`) has no single
// address to add.
let bind_ip = (!bind.is_unspecified()).then(|| canonical(bind));
Some(AllowedHosts {
port,
bind_ip,
extra_hosts,
})
} else {
None
}
}
/// Collapses an IPv4-mapped IPv6 address (`::ffff:192.168.0.1`) to its IPv4 form
/// so it classifies and compares consistently with the bare IPv4 address. Shared
/// with the `/api/open` peer check so it recognizes a mapped loopback peer too.
pub(crate) fn canonical(ip: IpAddr) -> IpAddr {
match ip {
IpAddr::V6(v6) => v6.to_ipv4_mapped().map(IpAddr::V4).unwrap_or(ip),
v4 => v4,
}
}
/// Returns whether a bind address is a specific private/link-local address, the
/// case where enforcement stays on with the bind IP added to the allowlist.
/// Unspecified, loopback, and public addresses are excluded (loopback is handled
/// separately by [`allowed_hosts`]).
fn is_private_bind(ip: IpAddr) -> bool {
match canonical(ip) {
IpAddr::V4(v4) => v4.is_private() || v4.is_link_local(),
IpAddr::V6(v6) => {
let first = v6.segments()[0];
// Unique local (fc00::/7) or link-local (fe80::/10). The std methods
// for these are still nightly-only, so test the prefix directly.
(first & 0xfe00) == 0xfc00 || (first & 0xffc0) == 0xfe80
}
}
}
/// Validates the `Host` and `Origin` headers of an incoming request against the
/// allowlist. Returns `Some` with a ready-to-send `404` response when the
/// request should be rejected, or `None` when it is allowed to proceed. When
/// `allowed` is `None` (unspecified or public bind) every request is accepted.
pub fn check_request_origin(
request: &Request<Body>,
allowed: Option<&AllowedHosts>,
) -> Option<Response<Body>> {
let allowed = allowed?;
// The Host header is mandatory and must refer to a local address.
let host_ok = request
.headers()
.get(HOST)
.and_then(|value| value.to_str().ok())
.and_then(parse_authority)
.is_some_and(|(host, port)| allowed.allows(&host, port));
if !host_ok {
return Some(reject());
}
// The Origin header is optional: non-browser clients such as the Roblox
// plugin never send it. When it is present (i.e. a browser made the request)
// it must also be local, which rejects a rebound page whose origin is still
// its own non-local hostname.
if let Some(origin) = request.headers().get(ORIGIN) {
let origin_ok = origin
.to_str()
.ok()
.and_then(parse_origin)
.is_some_and(|(host, port)| allowed.allows(&host, port));
if !origin_ok {
return Some(reject());
}
}
None
}
/// Normalizes a host literal for parsing/comparison: strips the surrounding
/// brackets from an IPv6 literal (e.g. `[::1]`) and drops any IPv6 zone id (e.g.
/// `fe80::1%eth0`), which `Ipv6Addr::from_str` would otherwise reject.
fn normalize_host(host: &str) -> &str {
let host = host
.strip_prefix('[')
.and_then(|host| host.strip_suffix(']'))
.unwrap_or(host);
host.split('%').next().unwrap_or(host)
}
/// Parses a `Host` header value (an authority such as `localhost:34872`) into
/// its host and optional port. Returns `None` if the authority carries userinfo
/// (e.g. `evil.com@localhost`), so a value whose host looks local only after the
/// userinfo is stripped can never sneak past the allowlist.
fn parse_authority(value: &str) -> Option<(String, Option<u16>)> {
let authority: Authority = value.parse().ok()?;
reject_userinfo(&authority)?;
Some((authority.host().to_owned(), authority.port_u16()))
}
/// Parses an `Origin` header value (an absolute URI such as
/// `http://localhost:34872`) into its host and optional port. Returns `None` for
/// origins without a host, such as the opaque `null` origin, or for origins whose
/// authority carries userinfo (see [`parse_authority`]).
fn parse_origin(value: &str) -> Option<(String, Option<u16>)> {
let uri: Uri = value.parse().ok()?;
reject_userinfo(uri.authority()?)?;
Some((uri.host()?.to_owned(), uri.port_u16()))
}
/// Returns `None` (rejecting the value) when an authority contains a userinfo
/// component, identified by the `@` separator. A bare host or `host:port` never
/// contains `@`, so this only fires on `userinfo@host` forms.
fn reject_userinfo(authority: &Authority) -> Option<()> {
if authority.as_str().contains('@') {
None
} else {
Some(())
}
}
/// Builds the response sent when a request fails Host/Origin validation. It is a
/// generic `404` with no Rojo-identifying body: a rejected request may be a
/// prober (or a DNS-rebound page's same-origin script, which could read the
/// body), so we reveal nothing rather than confirming a Rojo server is here.
fn reject() -> Response<Body> {
response(StatusCode::NOT_FOUND, "text/plain", "Not Found")
}
#[cfg(test)]
mod test {
use super::*;
use std::net::{Ipv4Addr, Ipv6Addr};
const PORT: u16 = 34872;
fn loopback_allowlist() -> Option<AllowedHosts> {
allowed_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), PORT, &[])
}
/// An allowlist for a server bound to a specific private (LAN) address.
fn private_allowlist() -> Option<AllowedHosts> {
allowed_hosts(IpAddr::V4(Ipv4Addr::new(192, 168, 1, 5)), PORT, &[])
}
/// An allowlist for `bind` with the given extra allowed hosts.
fn allowlist_with_hosts(bind: IpAddr, hosts: &[&str]) -> Option<AllowedHosts> {
let hosts: Vec<String> = hosts.iter().map(|host| host.to_string()).collect();
allowed_hosts(bind, PORT, &hosts)
}
fn request_with(headers: &[(&'static str, &str)]) -> Request<Body> {
let mut builder = Request::builder().uri("/api/rojo");
for (name, value) in headers {
builder = builder.header(*name, *value);
}
builder.body(Body::empty()).unwrap()
}
#[test]
fn loopback_bind_enables_enforcement() {
assert!(allowed_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), PORT, &[]).is_some());
assert!(allowed_hosts(IpAddr::V6(Ipv6Addr::LOCALHOST), PORT, &[]).is_some());
}
#[test]
fn private_bind_enables_enforcement() {
for bind in [
IpAddr::V4(Ipv4Addr::new(192, 168, 1, 5)), // 192.168.0.0/16
IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)), // 10.0.0.0/8
IpAddr::V4(Ipv4Addr::new(172, 16, 0, 1)), // 172.16.0.0/12
IpAddr::V4(Ipv4Addr::new(169, 254, 0, 1)), // link-local
IpAddr::V6(Ipv6Addr::new(0xfc00, 0, 0, 0, 0, 0, 0, 1)), // unique local
IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)), // link-local
] {
assert!(
allowed_hosts(bind, PORT, &[]).is_some(),
"private bind {bind} should enable enforcement"
);
}
}
#[test]
fn unspecified_or_public_bind_disables_enforcement() {
for bind in [
IpAddr::V4(Ipv4Addr::UNSPECIFIED), // 0.0.0.0
IpAddr::V6(Ipv6Addr::UNSPECIFIED), // ::
IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)), // public
IpAddr::V6(Ipv6Addr::new(0x2001, 0x4860, 0, 0, 0, 0, 0, 0x8888)), // public
] {
assert!(
allowed_hosts(bind, PORT, &[]).is_none(),
"bind {bind} should disable enforcement"
);
}
}
#[test]
fn accepts_local_hosts() {
let allowed = loopback_allowlist();
for host in [
format!("localhost:{PORT}"),
format!("127.0.0.1:{PORT}"),
format!("[::1]:{PORT}"),
"localhost".to_owned(),
] {
let request = request_with(&[("host", &host)]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_none(),
"host {host} should be allowed"
);
}
}
#[test]
fn rejects_foreign_host() {
let allowed = loopback_allowlist();
let request = request_with(&[("host", "evil.com")]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_wrong_port() {
let allowed = loopback_allowlist();
let request = request_with(&[("host", "localhost:1234")]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_missing_host() {
let allowed = loopback_allowlist();
let request = request_with(&[]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_host_with_userinfo() {
let allowed = loopback_allowlist();
// The host parses to `localhost`, but the userinfo prefix must keep it
// from being treated as a local address.
let request = request_with(&[("host", &format!("evil.com@localhost:{PORT}"))]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_origin_with_userinfo() {
let allowed = loopback_allowlist();
let request = request_with(&[
("host", &format!("localhost:{PORT}")),
("origin", &format!("http://evil.com@localhost:{PORT}")),
]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_foreign_origin_even_with_local_host() {
let allowed = loopback_allowlist();
let request = request_with(&[
("host", &format!("localhost:{PORT}")),
("origin", &format!("http://evil.com:{PORT}")),
]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn rejects_null_origin() {
let allowed = loopback_allowlist();
let request = request_with(&[("host", &format!("localhost:{PORT}")), ("origin", "null")]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn accepts_local_origin() {
let allowed = loopback_allowlist();
let request = request_with(&[
("host", &format!("localhost:{PORT}")),
("origin", &format!("http://localhost:{PORT}")),
]);
assert!(check_request_origin(&request, allowed.as_ref()).is_none());
}
#[test]
fn private_bind_accepts_local_and_bind_ip_hosts() {
let allowed = private_allowlist();
for host in [
format!("192.168.1.5:{PORT}"),
format!("localhost:{PORT}"),
format!("127.0.0.1:{PORT}"),
format!("[::1]:{PORT}"),
"192.168.1.5".to_owned(),
] {
let request = request_with(&[("host", &host)]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_none(),
"host {host} should be allowed on a private bind"
);
}
}
#[test]
fn private_bind_rejects_other_hosts() {
let allowed = private_allowlist();
for host in [
"evil.com", // a rebound attacker domain
"192.168.1.6", // a different private IP
"8.8.8.8", // a public IP
] {
let request = request_with(&[("host", host)]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_some(),
"host {host} should be rejected on a private bind"
);
}
}
#[test]
fn private_bind_keeps_origin_strict() {
// A different private IP as Origin must be rejected even though the Host
// is the valid bind IP: the Origin check is not widened to arbitrary
// private addresses.
let allowed = private_allowlist();
let request = request_with(&[
("host", &format!("192.168.1.5:{PORT}")),
("origin", &format!("http://192.168.1.6:{PORT}")),
]);
assert!(check_request_origin(&request, allowed.as_ref()).is_some());
}
#[test]
fn private_bind_accepts_bind_ip_origin() {
let allowed = private_allowlist();
let request = request_with(&[
("host", &format!("192.168.1.5:{PORT}")),
("origin", &format!("http://192.168.1.5:{PORT}")),
]);
assert!(check_request_origin(&request, allowed.as_ref()).is_none());
}
#[test]
fn accepts_ipv4_mapped_bind_ip() {
// `::ffff:192.168.1.5` is the IPv4-mapped form of the bind IP and must
// be treated as equal to it.
let allowed = private_allowlist();
let request = request_with(&[("host", &format!("[::ffff:192.168.1.5]:{PORT}"))]);
assert!(check_request_origin(&request, allowed.as_ref()).is_none());
}
#[test]
fn canonical_collapses_ipv4_mapped_loopback() {
// The `/api/open` peer gate relies on this: an IPv4 loopback client
// reaching a dual-stack (`::`) bind arrives as `::ffff:127.0.0.1`, which
// is only recognized as loopback after canonicalization.
let mapped: IpAddr = "::ffff:127.0.0.1".parse().unwrap();
assert!(!mapped.is_loopback());
assert!(canonical(mapped).is_loopback());
}
#[test]
fn normalize_host_strips_brackets_and_zone_id() {
// Brackets and an IPv6 zone id must be removed so the result parses as an
// `IpAddr` (`Ipv6Addr::from_str` rejects zone ids).
assert_eq!(normalize_host("[::1]"), "::1");
assert_eq!(normalize_host("[fe80::1%eth0]"), "fe80::1");
assert_eq!(normalize_host("localhost"), "localhost");
assert!(normalize_host("[fe80::1%eth0]").parse::<IpAddr>().is_ok());
}
#[test]
fn allowed_hosts_extend_the_allowlist() {
// A hostname listed as an allowed host is accepted on a loopback bind,
// with the usual port check still applied; an unlisted host is not.
let allowed = allowlist_with_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), &["mypc.lan"]);
let ok = request_with(&[("host", &format!("mypc.lan:{PORT}"))]);
assert!(check_request_origin(&ok, allowed.as_ref()).is_none());
let wrong_port = request_with(&[("host", "mypc.lan:1234")]);
assert!(check_request_origin(&wrong_port, allowed.as_ref()).is_some());
let other = request_with(&[("host", &format!("other.lan:{PORT}"))]);
assert!(check_request_origin(&other, allowed.as_ref()).is_some());
}
#[test]
fn allowed_hosts_apply_to_origin() {
let allowed = allowlist_with_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), &["mypc.lan"]);
let ok = request_with(&[
("host", &format!("mypc.lan:{PORT}")),
("origin", &format!("http://mypc.lan:{PORT}")),
]);
assert!(check_request_origin(&ok, allowed.as_ref()).is_none());
let foreign_origin = request_with(&[
("host", &format!("mypc.lan:{PORT}")),
("origin", &format!("http://evil.com:{PORT}")),
]);
assert!(check_request_origin(&foreign_origin, allowed.as_ref()).is_some());
}
#[test]
fn allowed_hosts_enable_enforcement_on_exposed_bind() {
// Binding to 0.0.0.0 normally disables validation, but listing a host
// turns it back on: localhost and the listed host are accepted while
// everything else is rejected.
let allowed = allowlist_with_hosts(IpAddr::V4(Ipv4Addr::UNSPECIFIED), &["mypc.lan"]);
assert!(allowed.is_some());
for host in [format!("mypc.lan:{PORT}"), format!("localhost:{PORT}")] {
let request = request_with(&[("host", &host)]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_none(),
"host {host} should be allowed"
);
}
let evil = request_with(&[("host", "evil.com")]);
assert!(check_request_origin(&evil, allowed.as_ref()).is_some());
}
#[test]
fn allowed_hosts_on_public_bind_accept_the_bind_ip() {
// A specific public bind with an allowed host accepts localhost, the
// listed host, and the bind IP itself, but nothing else. (203.0.113.0/24
// is the TEST-NET-3 documentation range, treated here as a public IP.)
let bind = IpAddr::V4(Ipv4Addr::new(203, 0, 113, 5));
let allowed = allowlist_with_hosts(bind, &["mypc.lan"]);
for host in [
format!("203.0.113.5:{PORT}"),
format!("mypc.lan:{PORT}"),
format!("localhost:{PORT}"),
] {
let request = request_with(&[("host", &host)]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_none(),
"host {host} should be allowed"
);
}
let evil = request_with(&[("host", "evil.com")]);
assert!(check_request_origin(&evil, allowed.as_ref()).is_some());
}
#[test]
fn disabled_allowlist_accepts_foreign_host() {
for bind in [
IpAddr::V4(Ipv4Addr::UNSPECIFIED),
IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)),
] {
let allowed = allowed_hosts(bind, PORT, &[]);
let request = request_with(&[("host", "evil.com")]);
assert!(
check_request_origin(&request, allowed.as_ref()).is_none(),
"disabled allowlist for bind {bind} should accept any host"
);
}
}
}

View File

@@ -6,7 +6,7 @@
use std::{borrow::Cow, sync::Arc, time::Duration};
use hyper::{header, Body, Method, Request, Response, StatusCode};
use hyper::{Body, Method, Request, Response, StatusCode};
use rbx_dom_weak::types::{Ref, Variant};
use ritz::{html, Fragment, HtmlContent, HtmlSelfClosingTag};
@@ -16,7 +16,7 @@ use crate::{
web::{
assets,
interface::{ErrorResponse, SERVER_VERSION},
util::json,
util::{json, response},
},
};
@@ -45,17 +45,11 @@ impl UiService {
}
fn handle_logo(&self) -> Response<Body> {
Response::builder()
.header(header::CONTENT_TYPE, "image/png")
.body(Body::from(assets::logo()))
.unwrap()
response(StatusCode::OK, "image/png", assets::logo())
}
fn handle_icon(&self) -> Response<Body> {
Response::builder()
.header(header::CONTENT_TYPE, "image/png")
.body(Body::from(assets::icon()))
.unwrap()
response(StatusCode::OK, "image/png", assets::icon())
}
fn handle_home(&self) -> Response<Body> {
@@ -66,10 +60,11 @@ impl UiService {
</div>
});
Response::builder()
.header(header::CONTENT_TYPE, "text/html")
.body(Body::from(format!("<!DOCTYPE html>{}", page)))
.unwrap()
response(
StatusCode::OK,
"text/html",
format!("<!DOCTYPE html>{}", page),
)
}
fn handle_show_instances(&self) -> Response<Body> {
@@ -80,10 +75,11 @@ impl UiService {
{ Self::instance(&tree, root_id) }
});
Response::builder()
.header(header::CONTENT_TYPE, "text/html")
.body(Body::from(format!("<!DOCTYPE html>{}", page)))
.unwrap()
response(
StatusCode::OK,
"text/html",
format!("<!DOCTYPE html>{}", page),
)
}
fn instance(tree: &RojoTree, id: Ref) -> HtmlContent<'_> {

Some files were not shown because too many files have changed in this diff Show More