diff --git a/internal/apimodule/admin/admin.go b/internal/apimodule/admin/admin.go index 2ebe9c7a7..200e7bd88 100644 --- a/internal/apimodule/admin/admin.go +++ b/internal/apimodule/admin/admin.go @@ -34,7 +34,7 @@ import ( const ( // BasePath is the base API path for this module - BasePath = "/api/v1/admin" + BasePath = "/api/v1/admin" // EmojiPath is used for posting/deleting custom emojis EmojiPath = BasePath + "/custom_emojis" ) diff --git a/internal/apimodule/auth/auth.go b/internal/apimodule/auth/auth.go index 341805b40..0a6788c37 100644 --- a/internal/apimodule/auth/auth.go +++ b/internal/apimodule/auth/auth.go @@ -32,9 +32,9 @@ import ( const ( // AuthSignInPath is the API path for users to sign in through - AuthSignInPath = "/auth/sign_in" + AuthSignInPath = "/auth/sign_in" // OauthTokenPath is the API path to use for granting token requests to users with valid credentials - OauthTokenPath = "/oauth/token" + OauthTokenPath = "/oauth/token" // OauthAuthorizePath is the API path for authorization requests (eg., authorize this app to act on my behalf as a user) OauthAuthorizePath = "/oauth/authorize" ) diff --git a/internal/apimodule/fileserver/fileserver.go b/internal/apimodule/fileserver/fileserver.go index 7651c8cc1..55b8781b6 100644 --- a/internal/apimodule/fileserver/fileserver.go +++ b/internal/apimodule/fileserver/fileserver.go @@ -39,7 +39,7 @@ const ( // MediaSizeKey is the url key for the desired media size--original/small/static MediaSizeKey = "media_size" // FileNameKey is the actual filename being sought. Will usually be a UUID then something like .jpeg - FileNameKey = "file_name" + FileNameKey = "file_name" ) // FileServer implements the RESTAPIModule interface. diff --git a/internal/apimodule/status/status.go b/internal/apimodule/status/status.go index 73a1b5847..c54ac4bcf 100644 --- a/internal/apimodule/status/status.go +++ b/internal/apimodule/status/status.go @@ -37,9 +37,9 @@ import ( const ( // IDKey is for status UUIDs - IDKey = "id" + IDKey = "id" // BasePath is the base path for serving the status API - BasePath = "/api/v1/statuses" + BasePath = "/api/v1/statuses" // BasePathWithID is just the base path with the ID key in it. // Use this anywhere you need to know the ID of the status being queried. BasePathWithID = BasePath + "/:" + IDKey @@ -48,31 +48,31 @@ const ( ContextPath = BasePathWithID + "/context" // FavouritedPath is for seeing who's faved a given status - FavouritedPath = BasePathWithID + "/favourited_by" + FavouritedPath = BasePathWithID + "/favourited_by" // FavouritePath is for posting a fave on a status - FavouritePath = BasePathWithID + "/favourite" + FavouritePath = BasePathWithID + "/favourite" // UnfavouritePath is for removing a fave from a status UnfavouritePath = BasePathWithID + "/unfavourite" // RebloggedPath is for seeing who's boosted a given status RebloggedPath = BasePathWithID + "/reblogged_by" // ReblogPath is for boosting/reblogging a given status - ReblogPath = BasePathWithID + "/reblog" + ReblogPath = BasePathWithID + "/reblog" // UnreblogPath is for undoing a boost/reblog of a given status - UnreblogPath = BasePathWithID + "/unreblog" + UnreblogPath = BasePathWithID + "/unreblog" // BookmarkPath is for creating a bookmark on a given status - BookmarkPath = BasePathWithID + "/bookmark" + BookmarkPath = BasePathWithID + "/bookmark" // UnbookmarkPath is for removing a bookmark from a given status UnbookmarkPath = BasePathWithID + "/unbookmark" // MutePath is for muting a given status so that notifications will no longer be received about it. - MutePath = BasePathWithID + "/mute" + MutePath = BasePathWithID + "/mute" // UnmutePath is for undoing an existing mute UnmutePath = BasePathWithID + "/unmute" // PinPath is for pinning a status to an account profile so that it's the first thing people see - PinPath = BasePathWithID + "/pin" + PinPath = BasePathWithID + "/pin" // UnpinPath is for undoing a pin and returning a status to the ever-swirling drain of time and entropy UnpinPath = BasePathWithID + "/unpin" ) diff --git a/internal/db/federating_db.go b/internal/db/federating_db.go index 16e3262ae..f25d32ed7 100644 --- a/internal/db/federating_db.go +++ b/internal/db/federating_db.go @@ -21,12 +21,16 @@ package db import ( "context" "errors" + "fmt" "net/url" "sync" "github.com/go-fed/activity/pub" "github.com/go-fed/activity/streams/vocab" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/util" ) // FederatingDB uses the underlying DB interface to implement the go-fed pub.Database interface. @@ -35,13 +39,15 @@ type federatingDB struct { locks *sync.Map db DB config *config.Config + log *logrus.Entry } -func newFederatingDB(db DB, config *config.Config) pub.Database { +func newFederatingDB(db DB, config *config.Config, log *logrus.Entry) pub.Database { return &federatingDB{ locks: new(sync.Map), db: db, config: config, + log: log, } } @@ -118,11 +124,75 @@ func (f *federatingDB) SetInbox(c context.Context, inbox vocab.ActivityStreamsOr return nil } -// Owns returns true if the database has an entry for the IRI and it -// exists in the database. -// +// Owns returns true if the IRI belongs to this instance, and if +// the database has an entry for the IRI. // The library makes this call only after acquiring a lock first. -func (f *federatingDB) Owns(c context.Context, id *url.URL) (owns bool, err error) { +func (f *federatingDB) Owns(c context.Context, id *url.URL) (bool, error) { + l := f.log.WithFields(logrus.Fields{ + "func": "Owns", + "activityID": id.String(), + }) + + // if the id host isn't this instance host, we don't own this IRI + if id.Host != f.config.Host { + return false, nil + } + + // apparently we own it, so what *is* it? + + // check if it's a status, eg /users/example_username/statuses/SOME_UUID_OF_A_STATUS + if util.IsStatusesPath(id) { + username, uid, err := util.ParseStatusesPath(id) + if err != nil { + return false, fmt.Errorf("error parsing statuses path for url %s: %s", id.String(), err) + } + acct := >smodel.Account{} + if err := f.db.GetWhere("username", username, acct); err != nil { + if _, ok := err.(ErrNoEntries); ok { + // there are no entries for this username + return false, nil + } + return false, fmt.Errorf("database error fetching account with username %s: %s", username, err) + } + if acct.Domain != "" { + // this is a remote account so we don't own it after all + return false, nil + } + status := >smodel.Status{} + if err := f.db.GetByID(uid, status); err != nil { + if _, ok := err.(ErrNoEntries); ok { + // there are no entries for this status + return false, nil + } + return false, fmt.Errorf("database error fetching status with id %s: %s", uid, err) + } + // the user exists, the status exists, we own both, we're good + return true, nil + } + + // check if it's a user, eg /users/example_username + if util.IsUserPath(id) { + username, err := util.ParseUserPath(id) + if err != nil { + return false, fmt.Errorf("error parsing statuses path for url %s: %s", id.String(), err) + } + acct := >smodel.Account{} + if err := f.db.GetWhere("username", username, acct); err != nil { + if _, ok := err.(ErrNoEntries); ok { + // there are no entries for this username + return false, nil + } + return false, fmt.Errorf("database error fetching account with username %s: %s", username, err) + } + if acct.Domain != "" { + // this is a remote account so we don't own it after all + return false, nil + } + // the user exists, we own it, we're good + return true, nil + } + + l.Info("could not match activityID") return false, nil } diff --git a/internal/db/pg.go b/internal/db/pg.go index e8a4b4004..4353be2d8 100644 --- a/internal/db/pg.go +++ b/internal/db/pg.go @@ -95,7 +95,7 @@ func newPostgresService(ctx context.Context, c *config.Config, log *logrus.Entry cancel: cancel, } - federatingDB := newFederatingDB(ps, c) + federatingDB := newFederatingDB(ps, c, log) ps.federationDB = federatingDB // we can confidently return this useable postgres service now diff --git a/internal/util/regexes.go b/internal/util/regexes.go index 60b397d86..db8124cdb 100644 --- a/internal/util/regexes.go +++ b/internal/util/regexes.go @@ -18,19 +18,70 @@ package util -import "regexp" +import ( + "fmt" + "regexp" +) + +const ( + minimumPasswordEntropy = 60 // dictates password strength. See https://github.com/wagslane/go-password-validator + minimumReasonLength = 40 + maximumReasonLength = 500 + maximumEmailLength = 256 + maximumUsernameLength = 64 + maximumPasswordLength = 64 + maximumEmojiShortcodeLength = 30 + maximumHashtagLength = 30 +) var ( // mention regex can be played around with here: https://regex101.com/r/qwM9D3/1 - mentionRegexString = `(?: |^|\W)(@[a-zA-Z0-9_]+(?:@[a-zA-Z0-9_\-\.]+)?)(?: |\n)` - mentionRegex = regexp.MustCompile(mentionRegexString) + mentionFinderRegexString = `(?: |^|\W)(@[a-zA-Z0-9_]+(?:@[a-zA-Z0-9_\-\.]+)?)(?: |\n)` + mentionFinderRegex = regexp.MustCompile(mentionFinderRegexString) + // hashtag regex can be played with here: https://regex101.com/r/Vhy8pg/1 - hashtagRegexString = `(?: |^|\W)?#([a-zA-Z0-9]{1,30})(?:\b|\r)` - hashtagRegex = regexp.MustCompile(hashtagRegexString) - // emoji regex can be played with here: https://regex101.com/r/478XGM/1 - emojiRegexString = `(?: |^|\W)?:([a-zA-Z0-9_]{2,30}):(?:\b|\r)?` - emojiRegex = regexp.MustCompile(emojiRegexString) + hashtagFinderRegexString = fmt.Sprintf(`(?: |^|\W)?#([a-zA-Z0-9]{1,%d})(?:\b|\r)`, maximumHashtagLength) + hashtagFinderRegex = regexp.MustCompile(hashtagFinderRegexString) + // emoji shortcode regex can be played with here: https://regex101.com/r/zMDRaG/1 - emojiShortcodeString = `^[a-z0-9_]{2,30}$` - emojiShortcodeRegex = regexp.MustCompile(emojiShortcodeString) + emojiShortcodeRegexString = fmt.Sprintf(`[a-z0-9_]{2,%d}`, maximumEmojiShortcodeLength) + emojiShortcodeValidationRegex = regexp.MustCompile(fmt.Sprintf("^%s$", emojiShortcodeRegexString)) + + // emoji regex can be played with here: https://regex101.com/r/478XGM/1 + emojiFinderRegexString = fmt.Sprintf(`(?: |^|\W)?:(%s):(?:\b|\r)?`, emojiShortcodeRegexString) + emojiFinderRegex = regexp.MustCompile(emojiFinderRegexString) + + // usernameRegexString defines an acceptable username on this instance + usernameRegexString = fmt.Sprintf(`[a-z0-9_]{2,%d}`, maximumUsernameLength) + // usernameValidationRegex can be used to validate usernames of new signups + usernameValidationRegex = regexp.MustCompile(fmt.Sprintf(`^%s$`, usernameRegexString)) + + userPathRegexString = fmt.Sprintf(`^?/%s/(%s)$`, UsersPath, usernameRegexString) + // userPathRegex parses a path that validates and captures the username part from eg /users/example_username + userPathRegex = regexp.MustCompile(userPathRegexString) + + actorPathRegexString = fmt.Sprintf(`^?/%s/(%s)$`, ActorsPath, usernameRegexString) + // actorPathRegex parses a path that validates and captures the username part from eg /actors/example_username + actorPathRegex = regexp.MustCompile(actorPathRegexString) + + followersPathRegexString = fmt.Sprintf(`^/?%s/(%s)/%s$`, UsersPath, usernameRegexString, FollowersPath) + // followersPathRegex parses a path that validates and captures the username part from eg /users/example_username/followers + followersPathRegex = regexp.MustCompile(followersPathRegexString) + + followingPathRegexString = fmt.Sprintf(`^/?%s/(%s)/%s$`, UsersPath, usernameRegexString, FollowingPath) + // followingPathRegex parses a path that validates and captures the username part from eg /users/example_username/following + followingPathRegex = regexp.MustCompile(followingPathRegexString) + + likedPathRegexString = fmt.Sprintf(`^/?%s/%s/%s$`, UsersPath, usernameRegexString, LikedPath) + // followingPathRegex parses a path that validates and captures the username part from eg /users/example_username/liked + likedPathRegex = regexp.MustCompile(likedPathRegexString) + + // see https://ihateregex.io/expr/uuid/ + uuidRegexString = `[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}` + + statusesPathRegexString = fmt.Sprintf(`^/?%s/(%s)/%s/(%s)$`, UsersPath, usernameRegexString, StatusesPath, uuidRegexString) + // statusesPathRegex parses a path that validates and captures the username part and the uuid part + // from eg /users/example_username/statuses/123e4567-e89b-12d3-a456-426655440000. + // The regex can be played with here: https://regex101.com/r/G9zuxQ/1 + statusesPathRegex = regexp.MustCompile(statusesPathRegexString) ) diff --git a/internal/util/statustools.go b/internal/util/statustools.go index fd9bb9c24..5591f185a 100644 --- a/internal/util/statustools.go +++ b/internal/util/statustools.go @@ -31,7 +31,7 @@ import ( // The case of the returned mentions will be lowered, for consistency. func DeriveMentions(status string) []string { mentionedAccounts := []string{} - for _, m := range mentionRegex.FindAllStringSubmatch(status, -1) { + for _, m := range mentionFinderRegex.FindAllStringSubmatch(status, -1) { mentionedAccounts = append(mentionedAccounts, m[1]) } return lower(unique(mentionedAccounts)) @@ -43,7 +43,7 @@ func DeriveMentions(status string) []string { // tags will be lowered, for consistency. func DeriveHashtags(status string) []string { tags := []string{} - for _, m := range hashtagRegex.FindAllStringSubmatch(status, -1) { + for _, m := range hashtagFinderRegex.FindAllStringSubmatch(status, -1) { tags = append(tags, m[1]) } return lower(unique(tags)) @@ -55,7 +55,7 @@ func DeriveHashtags(status string) []string { // emojis will be lowered, for consistency. func DeriveEmojis(status string) []string { emojis := []string{} - for _, m := range emojiRegex.FindAllStringSubmatch(status, -1) { + for _, m := range emojiFinderRegex.FindAllStringSubmatch(status, -1) { emojis = append(emojis, m[1]) } return lower(unique(emojis)) diff --git a/internal/util/uri.go b/internal/util/uri.go index 8617c58c7..a4c7b8a48 100644 --- a/internal/util/uri.go +++ b/internal/util/uri.go @@ -21,44 +21,55 @@ package util import ( "fmt" "net/url" + "strings" ) const ( // UsersPath is for serving users info - UsersPath = "users" + UsersPath = "users" + // ActorsPath is for serving actors info + ActorsPath = "actors" // StatusesPath is for serving statuses - StatusesPath = "statuses" + StatusesPath = "statuses" // InboxPath represents the webfinger inbox location - InboxPath = "inbox" + InboxPath = "inbox" // OutboxPath represents the webfinger outbox location - OutboxPath = "outbox" + OutboxPath = "outbox" // FollowersPath represents the webfinger followers location - FollowersPath = "followers" + FollowersPath = "followers" + // FollowingPath represents the webfinger following location + FollowingPath = "following" + // LikedPath represents the webfinger liked location + LikedPath = "liked" // CollectionsPath represents the webfinger collections location CollectionsPath = "collections" // FeaturedPath represents the webfinger featured location - FeaturedPath = "featured" + FeaturedPath = "featured" ) // UserURIs contains a bunch of UserURIs and URLs for a user, host, account, etc. type UserURIs struct { // The web URL of the instance host, eg https://example.org - HostURL string + HostURL string // The web URL of the user, eg., https://example.org/@example_user - UserURL string + UserURL string // The web URL for statuses of this user, eg., https://example.org/@example_user/statuses StatusesURL string // The webfinger URI of this user, eg., https://example.org/users/example_user - UserURI string + UserURI string // The webfinger URI for this user's statuses, eg., https://example.org/users/example_user/statuses - StatusesURI string + StatusesURI string // The webfinger URI for this user's activitypub inbox, eg., https://example.org/users/example_user/inbox - InboxURI string + InboxURI string // The webfinger URI for this user's activitypub outbox, eg., https://example.org/users/example_user/outbox - OutboxURI string + OutboxURI string // The webfinger URI for this user's followers, eg., https://example.org/users/example_user/followers - FollowersURI string + FollowersURI string + // The webfinger URI for this user's following, eg., https://example.org/users/example_user/following + FollowingURI string + // The webfinger URI for this user's liked posts eg., https://example.org/users/example_user/liked + LikedURI string // The webfinger URI for this user's featured collections, eg., https://example.org/users/example_user/collections/featured CollectionURI string } @@ -76,6 +87,8 @@ func GenerateURIsForAccount(username string, protocol string, host string) *User inboxURI := fmt.Sprintf("%s/%s", userURI, InboxPath) outboxURI := fmt.Sprintf("%s/%s", userURI, OutboxPath) followersURI := fmt.Sprintf("%s/%s", userURI, FollowersPath) + followingURI := fmt.Sprintf("%s/%s", userURI, FollowingPath) + likedURI := fmt.Sprintf("%s/%s", userURI, LikedPath) collectionURI := fmt.Sprintf("%s/%s/%s", userURI, CollectionsPath, FeaturedPath) return &UserURIs{ HostURL: hostURL, @@ -87,10 +100,61 @@ func GenerateURIsForAccount(username string, protocol string, host string) *User InboxURI: inboxURI, OutboxURI: outboxURI, FollowersURI: followersURI, + FollowingURI: followingURI, + LikedURI: likedURI, CollectionURI: collectionURI, } } -func ParseActivityPubRequestURL(id *url.URL) error { - return nil +// IsUserPath returns true if the given URL path corresponds to eg /users/example_username +func IsUserPath(id *url.URL) bool { + return userPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// IsInstanceActorPath returns true if the given URL path corresponds to eg /actors/example_username +func IsInstanceActorPath(id *url.URL) bool { + return actorPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// IsFollowersPath returns true if the given URL path corresponds to eg /users/example_username/followers +func IsFollowersPath(id *url.URL) bool { + return followersPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// IsFollowingPath returns true if the given URL path corresponds to eg /users/example_username/following +func IsFollowingPath(id *url.URL) bool { + return followingPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// IsLikedPath returns true if the given URL path corresponds to eg /users/example_username/liked +func IsLikedPath(id *url.URL) bool { + return followingPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// IsStatusesPath returns true if the given URL path corresponds to eg /users/example_username/statuses/SOME_UUID_OF_A_STATUS +func IsStatusesPath(id *url.URL) bool { + return statusesPathRegex.MatchString(strings.ToLower(id.Path)) +} + +// ParseStatusesPath returns the username and uuid from a path such as /users/example_username/statuses/SOME_UUID_OF_A_STATUS +func ParseStatusesPath(id *url.URL) (username string, uuid string, err error) { + matches := statusesPathRegex.FindStringSubmatch(id.Path) + if len(matches) != 3 { + err = fmt.Errorf("expected 3 matches but matches length was %d", len(matches)) + return + } + username = matches[1] + uuid = matches[2] + return +} + +// ParseUserPath returns the username from a path such as /users/example_username +func ParseUserPath(id *url.URL) (username string, err error) { + matches := userPathRegex.FindStringSubmatch(id.Path) + if len(matches) != 2 { + err = fmt.Errorf("expected 2 matches but matches length was %d", len(matches)) + return + } + username = matches[1] + return } diff --git a/internal/util/validation.go b/internal/util/validation.go index acf0e68cd..d392231bb 100644 --- a/internal/util/validation.go +++ b/internal/util/validation.go @@ -22,45 +22,22 @@ import ( "errors" "fmt" "net/mail" - "regexp" pwv "github.com/wagslane/go-password-validator" "golang.org/x/text/language" ) -const ( - // MinimumPasswordEntropy dictates password strength. See https://github.com/wagslane/go-password-validator - MinimumPasswordEntropy = 60 - // MinimumReasonLength is the length of chars we expect as a bare minimum effort - MinimumReasonLength = 40 - // MaximumReasonLength is the maximum amount of chars we're happy to accept - MaximumReasonLength = 500 - // MaximumEmailLength is the maximum length of an email address we're happy to accept - MaximumEmailLength = 256 - // MaximumUsernameLength is the maximum length of a username we're happy to accept - MaximumUsernameLength = 64 - // MaximumPasswordLength is the maximum length of a password we're happy to accept - MaximumPasswordLength = 64 - // NewUsernameRegexString is string representation of the regular expression for validating usernames - NewUsernameRegexString = `^[a-z0-9_]+$` -) - -var ( - // NewUsernameRegex is the compiled regex for validating new usernames - NewUsernameRegex = regexp.MustCompile(NewUsernameRegexString) -) - // ValidateNewPassword returns an error if the given password is not sufficiently strong, or nil if it's ok. func ValidateNewPassword(password string) error { if password == "" { return errors.New("no password provided") } - if len(password) > MaximumPasswordLength { - return fmt.Errorf("password should be no more than %d chars", MaximumPasswordLength) + if len(password) > maximumPasswordLength { + return fmt.Errorf("password should be no more than %d chars", maximumPasswordLength) } - return pwv.Validate(password, MinimumPasswordEntropy) + return pwv.Validate(password, minimumPasswordEntropy) } // ValidateUsername makes sure that a given username is valid (ie., letters, numbers, underscores, check length). @@ -70,11 +47,11 @@ func ValidateUsername(username string) error { return errors.New("no username provided") } - if len(username) > MaximumUsernameLength { - return fmt.Errorf("username should be no more than %d chars but '%s' was %d", MaximumUsernameLength, username, len(username)) + if len(username) > maximumUsernameLength { + return fmt.Errorf("username should be no more than %d chars but '%s' was %d", maximumUsernameLength, username, len(username)) } - if !NewUsernameRegex.MatchString(username) { + if !usernameValidationRegex.MatchString(username) { return fmt.Errorf("given username %s was invalid: must contain only lowercase letters, numbers, and underscores", username) } @@ -88,8 +65,8 @@ func ValidateEmail(email string) error { return errors.New("no email provided") } - if len(email) > MaximumEmailLength { - return fmt.Errorf("email address should be no more than %d chars but '%s' was %d", MaximumEmailLength, email, len(email)) + if len(email) > maximumEmailLength { + return fmt.Errorf("email address should be no more than %d chars but '%s' was %d", maximumEmailLength, email, len(email)) } _, err := mail.ParseAddress(email) @@ -118,12 +95,12 @@ func ValidateSignUpReason(reason string, reasonRequired bool) error { return errors.New("no reason provided") } - if len(reason) < MinimumReasonLength { - return fmt.Errorf("reason should be at least %d chars but '%s' was %d", MinimumReasonLength, reason, len(reason)) + if len(reason) < minimumReasonLength { + return fmt.Errorf("reason should be at least %d chars but '%s' was %d", minimumReasonLength, reason, len(reason)) } - if len(reason) > MaximumReasonLength { - return fmt.Errorf("reason should be no more than %d chars but given reason was %d", MaximumReasonLength, len(reason)) + if len(reason) > maximumReasonLength { + return fmt.Errorf("reason should be no more than %d chars but given reason was %d", maximumReasonLength, len(reason)) } return nil } @@ -150,7 +127,7 @@ func ValidatePrivacy(privacy string) error { // for emoji shortcodes, to figure out whether it's a valid shortcode, ie., 2-30 characters, // lowercase a-z, numbers, and underscores. func ValidateEmojiShortcode(shortcode string) error { - if !emojiShortcodeRegex.MatchString(shortcode) { + if !emojiShortcodeValidationRegex.MatchString(shortcode) { return fmt.Errorf("shortcode %s did not pass validation, must be between 2 and 30 characters, lowercase letters, numbers, and underscores only", shortcode) } return nil