Bezpieczeństwo użytkowania Thread.current [] w szynach


81

Ciągle otrzymuję sprzeczne opinie na temat praktyki przechowywania informacji w Thread.currenthashu (np. Current_user, bieżąca subdomena itp.). Technika ta została zaproponowana jako sposób na uproszczenie późniejszego przetwarzania w warstwie modelu (określanie zakresu zapytań, audyt itp.).

Wielu uważa tę praktykę za niedopuszczalną, ponieważ łamie wzorzec MVC. Inni wyrażają obawy co do niezawodności / bezpieczeństwa podejścia, a moje 2-częściowe pytanie skupia się na tym drugim aspekcie.

  1. Czy Thread.currentgwarantowane jest, że hash będzie dostępny i prywatny dla jednej i tylko jednej odpowiedzi przez cały cykl?

  2. Rozumiem, że wątek na końcu odpowiedzi może zostać przekazany innym przychodzącym żądaniom, powodując wyciek wszelkich przechowywanych w nim informacji Thread.current. Czy wyczyszczenie takich informacji przed zakończeniem odpowiedzi (np. Poprzez wykonanie odpowiedzi Thread.current[:user] = nilod administratora after_filter) wystarczyłoby, aby zapobiec takiemu naruszeniu bezpieczeństwa?

Dzięki! Giuseppe


9
Sprawdź sekcję „Zabrudzenie z Thread.current” tutaj. m.onkey.org/thread-safety-for-your-rails . Napisał to jeden z jrubskich autorów. Sam kod ROR # 1 używa Thread.current dla I18N i time_zone. Czy to mówi o jego gwarancji? # 2. Jeśli # 1 jest prawdziwy, to wystarczy.
— so_mv

Dzięki, dodałem odniesienie.
— Giuseppe

3
W poście, do którego odsyłasz, przykłady dotyczą wyłącznie warstwy kontrolera, a zaproponowane rozwiązanie jest oczywiście odpowiednie. Podejrzewam jednak, że tym, czym zainteresowałaby się większość ludzi, byłby czysty sposób udostępniania modeli do 1-2 fragmentów informacji, które normalnie są dla nich niedostępne, bez dodawania dodatkowych parametrów do każdego wywołania modeli. Pod tym względem wszystkie te wielkie przerażające znaki ostrzegawcze „trzymaj się z daleka od Thread.current” bez konkretnych powodów, dla których dotychczas pozostawiały mnie niepewnym. Dzięki
— Giuseppe

Odpowiedzi:


48

Nie ma konkretnego powodu, aby trzymać się z daleka od zmiennych lokalnych wątku, główne problemy to:

  • trudniej je przetestować, ponieważ będziesz musiał pamiętać o ustawieniu zmiennych lokalnych wątku podczas testowania kodu, który go używa
  • klasy, które używają miejscowych wątków, będą potrzebowały wiedzy, że te obiekty nie są dla nich dostępne, ale znajdują się w zmiennej lokalnej wątku i tego rodzaju pośrednictwo zwykle łamie prawo demeter
  • brak czyszczenia ustawień lokalnych wątku może być problemem, jeśli środowisko ponownie wykorzystuje wątki (zmienna lokalna wątku byłaby już zainicjowana, a kod, na którym opiera się || = wywołania inicjalizacji zmiennych mogą się nie powieść

Tak więc, chociaż nie jest to całkowicie wykluczone, najlepszym podejściem jest ich nie używać, ale od czasu do czasu trafisz na ścianę, w której wątek lokalny będzie najprostszym możliwym rozwiązaniem bez zmiany dużej ilości kodu i będziesz musiał pójść na kompromis, mieć mniej niż doskonały model obiektowy z lokalnym wątkiem lub zmienić całkiem sporo kodu, aby zrobić to samo.

Jest to więc głównie kwestia przemyślenia, które będzie najlepszym rozwiązaniem dla twojego przypadku, a jeśli naprawdę podążasz ścieżką lokalną wątku, z pewnością radzę ci to zrobić z blokami, które pamiętają o posprzątaniu po są wykonywane, jak poniżej:

around_filter :do_with_current_user

def do_with_current_user
    Thread.current[:current_user] = self.current_user
    begin
        yield
    ensure
        Thread.current[:current_user] = nil
    end      
end

Dzięki temu zmienna lokalna wątku zostanie wyczyszczona przed użyciem, jeśli ten wątek zostanie odtworzony.


1
Używanie filtra dookoła z blokadą zapewniającą będzie bałagan przy obsłudze wyjątków / rejestrowaniu, jeśli nie jesteś zbyt ostrożny. Sprawdź RequestStore w odpowiedzi Dejana, aby uzyskać bardziej przejrzyste rozwiązanie oparte na oprogramowaniu pośrednim.
— Irongaze.com

Być może moje podejście do problemu polegało na tym, że najpierw ratowałem wyjątek (a nie tylko upewniając się, jak powyżej). Stos wywołań wyjątku był resetowany do lokalizacji, w której ponownie zgłosiłem, psując dziennik błędów Railsów i gdzie indziej. Skończyło się na tym, że ręcznie zrzuciłem przechwycony wyjątek do pliku dziennika, aby go zachować, ale był brzydki. Korzystanie z podejścia opartego na oprogramowaniu pośrednim jest znacznie czystszym IMHO.
— Irongaze.com

1
Dlatego powyższy kod ma tylko ensureblok i nie próbuje przechwycić wyjątku.
— Maurício Linhares

Proste, kiedy to widzisz :)
— 244an



4

Przyjęta odpowiedź jest technicznie dokładna, ale jak wskazano w odpowiedzi delikatnie, a w http://m.onkey.org/thread-safety-for-your-rails nie tak delikatnie:

Nie używaj lokalnego magazynu wątków, Thread.currentjeśli absolutnie nie musisz

Klejnotem request_storejest inne rozwiązanie (lepsze), ale po prostu przeczytaj plik Readme, aby uzyskać więcej powodów, aby trzymać się z daleka od lokalnego magazynu wątków.

Prawie zawsze jest lepszy sposób.


3
Używam Unicorn z aplikacją Rails Multi-tenant. Czy możesz zasugerować przykład „lepszego sposobu”? Opublikowany link zgłasza błąd aplikacji. Dzięki.
— lacostenycoder
Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.