From a3a228862c171e0fb27df11e7f060c90f20fcf30 Mon Sep 17 00:00:00 2001
From: Tom Hughes <tom@compton.nu>
Date: Thu, 9 Jun 2022 19:11:59 +0100
Subject: [PATCH] Add validation for relation member roles

Fixes #3563
---
 app/models/old_relation_member.rb   |  2 ++
 app/models/relation_member.rb       |  2 ++
 test/models/relation_member_test.rb | 20 ++++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/app/models/old_relation_member.rb b/app/models/old_relation_member.rb
index 5bc2e22e0..c99f60e7c 100644
--- a/app/models/old_relation_member.rb
+++ b/app/models/old_relation_member.rb
@@ -25,4 +25,6 @@ class OldRelationMember < ApplicationRecord
   belongs_to :old_relation, :foreign_key => [:relation_id, :version], :inverse_of => :old_members
   # A bit messy, referring to the current tables, should do for the data browser for now
   belongs_to :member, :polymorphic => true
+
+  validates :member_role, :allow_blank => true, :length => { :maximum => 255 }, :characters => true
 end
diff --git a/app/models/relation_member.rb b/app/models/relation_member.rb
index 2de551c3f..dd47bdcf6 100644
--- a/app/models/relation_member.rb
+++ b/app/models/relation_member.rb
@@ -23,4 +23,6 @@ class RelationMember < ApplicationRecord
 
   belongs_to :relation
   belongs_to :member, :polymorphic => true
+
+  validates :member_role, :allow_blank => true, :length => { :maximum => 255 }, :characters => true
 end
diff --git a/test/models/relation_member_test.rb b/test/models/relation_member_test.rb
index 170b1977c..a82cea457 100644
--- a/test/models/relation_member_test.rb
+++ b/test/models/relation_member_test.rb
@@ -1,4 +1,24 @@
 require "test_helper"
 
 class RelationMemberTest < ActiveSupport::TestCase
+  def test_role_with_invalid_characters
+    invalid = ["\x7f<hr/>", "test@example.com\x0e-", "s/\x1ff", "aa/\ufffe",
+               "aa\x0b-,", "aa?\x08", "/;\uffff.,?", "\x0c#ping",
+               "foo\x1fbar", "foo\x7fbar", "foo\ufffebar", "foo\uffffbar"]
+    relation = create(:relation)
+    node = create(:node)
+    invalid.each do |r|
+      member = build(:relation_member, :relation => relation, :member => node, :member_role => r)
+      assert_not member.valid?, "'#{r}' should not be valid"
+      assert_predicate member.errors[:member_role], :any?
+    end
+  end
+
+  def test_role_too_long
+    relation = create(:relation)
+    node = create(:node)
+    member = build(:relation_member, :relation => relation, :member => node, :member_role => "r" * 256)
+    assert_not member.valid?, "Role should be too long"
+    assert_predicate member.errors[:member_role], :any?
+  end
 end
-- 
2.39.5