Fix threaded news bugs

1. Fix handling of article IDs sent as 4 bytes instead of 2
2. Fix incorrect article count listing that strangely only affects third party clients

diff --git a/hotline/news.go b/hotline/news.go
index e25a2c8e1d3827040328437f5320566ec35ab438..11e2a27dc54ee9bcc729faf6b429ad943d09ef98 100644 (file)
--- a/hotline/news.go
+++ b/hotline/news.go
@@ -52,6 +52,7 @@ func (newscat *NewsCategoryListData15) GetNewsArtListData() NewsArtListData {
 
        nald := NewsArtListData{
                ID:          []byte{0, 0, 0, 0},
 
        nald := NewsArtListData{
                ID:          []byte{0, 0, 0, 0},

+               Count:       len(newsArts),

                Name:        []byte{},
                Description: []byte{},
                NewsArtList: newsArtsPayload,
                Name:        []byte{},
                Description: []byte{},
                NewsArtList: newsArtsPayload,


@@ -85,11 +86,12 @@ type NewsArtListData struct {
        Name        []byte `yaml:"Name"`
        Description []byte `yaml:"Description"` // not used?
        NewsArtList []byte // List of articles                  Optional (if article count > 0)
        Name        []byte `yaml:"Name"`
        Description []byte `yaml:"Description"` // not used?
        NewsArtList []byte // List of articles                  Optional (if article count > 0)

+       Count       int

 }
 
 func (nald *NewsArtListData) Payload() []byte {
        count := make([]byte, 4)
 }
 
 func (nald *NewsArtListData) Payload() []byte {
        count := make([]byte, 4)

-       binary.BigEndian.PutUint32(count, uint32(len(nald.NewsArtList)))
+       binary.BigEndian.PutUint32(count, uint32(nald.Count))

 
        out := append(nald.ID, count...)
        out = append(out, []byte{uint8(len(nald.Name))}...)
 
        out := append(nald.ID, count...)
        out = append(out, []byte{uint8(len(nald.Name))}...)

diff --git a/hotline/server.go b/hotline/server.go
index bda0c2a6f3b4d68b69d3a5adf6b9d9ec45783aef..d87ddb99ea121fd499695ad44c10ba6ab6ae74b1 100644 (file)
--- a/hotline/server.go
+++ b/hotline/server.go
@@ -36,6 +36,8 @@ type requestCtx struct {
 var nostalgiaVersion = []byte{0, 0, 2, 0x2c} // version ID used by the Nostalgia client
 var frogblastVersion = []byte{0, 0, 0, 0xb9} // version ID used by the Frogblast 1.2.4 client
 
 var nostalgiaVersion = []byte{0, 0, 2, 0x2c} // version ID used by the Nostalgia client
 var frogblastVersion = []byte{0, 0, 0, 0xb9} // version ID used by the Frogblast 1.2.4 client
 

+var heildrun = []byte{0, 0x97}
+

 var obsessionVersion = []byte{0xbe, 0x00} // version ID used by the Obsession client
 
 type Server struct {
 var obsessionVersion = []byte{0xbe, 0x00} // version ID used by the Obsession client
 
 type Server struct {


@@ -676,7 +678,7 @@ func (s *Server) handleNewConnection(ctx context.Context, rwc io.ReadWriteCloser
 
        // Used simplified hotline v1.2.3 login flow for clients that do not send login info in tranAgreed
        // TODO: figure out a generalized solution that doesn't require playing whack-a-mole for specific client versions
 
        // Used simplified hotline v1.2.3 login flow for clients that do not send login info in tranAgreed
        // TODO: figure out a generalized solution that doesn't require playing whack-a-mole for specific client versions

-       if c.Version == nil || bytes.Equal(c.Version, nostalgiaVersion) || bytes.Equal(c.Version, frogblastVersion) || bytes.Equal(c.Version, obsessionVersion) {
+       if c.Version == nil || bytes.Equal(c.Version, nostalgiaVersion) || bytes.Equal(c.Version, frogblastVersion) || bytes.Equal(c.Version, obsessionVersion) || bytes.Equal(c.Version, heildrun) {

                c.Agreed = true
                c.logger = c.logger.With("name", string(c.UserName))
                c.logger.Infow("Login successful", "clientVersion", fmt.Sprintf("%v", func() int { i, _ := byteToInt(c.Version); return i }()))
                c.Agreed = true
                c.logger = c.logger.With("name", string(c.UserName))
                c.logger.Infow("Login successful", "clientVersion", fmt.Sprintf("%v", func() int { i, _ := byteToInt(c.Version); return i }()))

diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go
index 7f8cbef4e99a6afc3e269875418432c40b1e1470..12b1d4151a49fc1914162730ab5b8ed7e7e0a2a1 100644 (file)
--- a/hotline/transaction_handlers.go
+++ b/hotline/transaction_handlers.go
@@ -287,6 +287,7 @@ func HandleChatSend(cc *ClientConn, t *Transaction) (res []Transaction, err erro
 
 // HandleSendInstantMsg sends instant message to the user on the current server.
 // Fields used in the request:
 
 // HandleSendInstantMsg sends instant message to the user on the current server.
 // Fields used in the request:

+//

 //     103     User ID
 //     113     Options
 //             One of the following values:
 //     103     User ID
 //     113     Options
 //             One of the following values:


@@ -897,17 +898,6 @@ func HandleUserBroadcast(cc *ClientConn, t *Transaction) (res []Transaction, err
        return res, err
 }
 
        return res, err
 }
 

-func byteToInt(bytes []byte) (int, error) {
-       switch len(bytes) {
-       case 2:
-               return int(binary.BigEndian.Uint16(bytes)), nil
-       case 4:
-               return int(binary.BigEndian.Uint32(bytes)), nil
-       }
-
-       return 0, errors.New("unknown byte length")
-}
-

 // HandleGetClientInfoText returns user information for the specific user.
 //
 // Fields used in the request:
 // HandleGetClientInfoText returns user information for the specific user.
 //
 // Fields used in the request:


@@ -1198,10 +1188,12 @@ func HandleNewNewsFldr(cc *ClientConn, t *Transaction) (res []Transaction, err e
        return res, err
 }
 
        return res, err
 }
 

+// HandleGetNewsArtData gets the list of article names at the specified news path.
+

 // Fields used in the request:
 // 325 News path       Optional
 // Fields used in the request:
 // 325 News path       Optional

-//
-// Reply fields:
+
+// Fields used in the reply:

 // 321 News article list data  Optional
 func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        if !cc.Authorize(accessNewsReadArt) {
 // 321 News article list data  Optional
 func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        if !cc.Authorize(accessNewsReadArt) {


@@ -1224,47 +1216,51 @@ func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction
        return res, err
 }
 
        return res, err
 }
 

+// HandleGetNewsArtData requests information about the specific news article.
+// Fields used in the request:
+//
+// Request fields
+// 325 News path
+// 326 News article ID
+// 327 News article data flavor
+//
+// Fields used in the reply:
+// 328 News article title
+// 329 News article poster
+// 330 News article date
+// 331 Previous article ID
+// 332 Next article ID
+// 335 Parent article ID
+// 336 First child article ID
+// 327 News article data flavor        "Should be “text/plain”
+// 333 News article data       Optional (if data flavor is “text/plain”)

 func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        if !cc.Authorize(accessNewsReadArt) {
                res = append(res, cc.NewErrReply(t, "You are not allowed to read news."))
                return res, err
        }
 
 func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        if !cc.Authorize(accessNewsReadArt) {
                res = append(res, cc.NewErrReply(t, "You are not allowed to read news."))
                return res, err
        }
 

-       // Request fields
-       // 325  News fp
-       // 326  News article ID
-       // 327  News article data flavor
-
-       pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data)
-

        var cat NewsCategoryListData15
        cats := cc.Server.ThreadedNews.Categories
 
        var cat NewsCategoryListData15
        cats := cc.Server.ThreadedNews.Categories
 

-       for _, fp := range pathStrs {
+       for _, fp := range ReadNewsPath(t.GetField(fieldNewsPath).Data) {

                cat = cats[fp]
                cats = cats[fp].SubCats
        }
                cat = cats[fp]
                cats = cats[fp].SubCats
        }

-       newsArtID := t.GetField(fieldNewsArtID).Data

 
 

-       convertedArtID := binary.BigEndian.Uint16(newsArtID)
+       // The official Hotline clients will send the article ID as 2 bytes if possible, but
+       // some third party clients such as Frogblast and Heildrun will always send 4 bytes
+       convertedID, err := byteToInt(t.GetField(fieldNewsArtID).Data)
+       if err != nil {
+               return res, err
+       }

 
 

-       art := cat.Articles[uint32(convertedArtID)]
+       art := cat.Articles[uint32(convertedID)]

        if art == nil {
                res = append(res, cc.NewReply(t))
                return res, err
        }
 
        if art == nil {
                res = append(res, cc.NewReply(t))
                return res, err
        }
 

-       // Reply fields
-       // 328  News article title
-       // 329  News article poster
-       // 330  News article date
-       // 331  Previous article ID
-       // 332  Next article ID
-       // 335  Parent article ID
-       // 336  First child article ID
-       // 327  News article data flavor        "Should be “text/plain”
-       // 333  News article data       Optional (if data flavor is “text/plain”)
-

        res = append(res, cc.NewReply(t,
                NewField(fieldNewsArtTitle, []byte(art.Title)),
                NewField(fieldNewsArtPoster, []byte(art.Poster)),
        res = append(res, cc.NewReply(t,
                NewField(fieldNewsArtTitle, []byte(art.Title)),
                NewField(fieldNewsArtPoster, []byte(art.Poster)),


@@ -1882,7 +1878,8 @@ func HandleJoinChat(cc *ClientConn, t *Transaction) (res []Transaction, err erro
 
 // HandleLeaveChat is sent from a v1.8+ Hotline client when the user exits a private chat
 // Fields used in the request:
 
 // HandleLeaveChat is sent from a v1.8+ Hotline client when the user exits a private chat
 // Fields used in the request:

-//     * 114   fieldChatID
+//   - 114     fieldChatID
+//

 // Reply is not expected.
 func HandleLeaveChat(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        chatID := t.GetField(fieldChatID).Data
 // Reply is not expected.
 func HandleLeaveChat(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        chatID := t.GetField(fieldChatID).Data

diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go
index 4dee0b58acf166711d037709bc15f43930f5c6d1..2720cddc07d9b82f007fb95fb70d6f3b707fa335 100644 (file)
--- a/hotline/transaction_handlers_test.go
+++ b/hotline/transaction_handlers_test.go
@@ -3684,3 +3684,58 @@ func TestHandleInviteNewChat(t *testing.T) {
                })
        }
 }
                })
        }
 }

+
+func TestHandleGetNewsArtData(t *testing.T) {
+       type args struct {
+               cc *ClientConn
+               t  *Transaction
+       }
+       tests := []struct {
+               name    string
+               args    args
+               wantRes []Transaction
+               wantErr assert.ErrorAssertionFunc
+       }{
+               {
+                       name: "when user does not have required permission",
+                       args: args{
+                               cc: &ClientConn{
+                                       Account: &Account{
+                                               Access: func() accessBitmap {
+                                                       var bits accessBitmap
+                                                       return bits
+                                               }(),
+                                       },
+                                       Server: &Server{
+                                               Accounts: map[string]*Account{},
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranGetNewsArtData, &[]byte{0, 1},
+                               ),
+                       },
+                       wantRes: []Transaction{
+                               {
+                                       Flags:     0x00,
+                                       IsReply:   0x01,
+                                       Type:      []byte{0, 0x00},
+                                       ID:        []byte{0x9a, 0xcb, 0x04, 0x42},
+                                       ErrorCode: []byte{0, 0, 0, 1},
+                                       Fields: []Field{
+                                               NewField(fieldError, []byte("You are not allowed to read news.")),
+                                       },
+                               },
+                       },
+                       wantErr: assert.NoError,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       gotRes, err := HandleGetNewsArtData(tt.args.cc, tt.args.t)
+                       if !tt.wantErr(t, err, fmt.Sprintf("HandleGetNewsArtData(%v, %v)", tt.args.cc, tt.args.t)) {
+                               return
+                       }
+                       tranAssertEqual(t, tt.wantRes, gotRes)
+               })
+       }
+}

diff --git a/hotline/util.go b/hotline/util.go
new file mode 100644 (file)
index 0000000..d37feb5
--- /dev/null
+++ b/hotline/util.go
@@ -0,0 +1,17 @@
+package hotline
+
+import (
+       "encoding/binary"
+       "errors"
+)
+
+func byteToInt(bytes []byte) (int, error) {
+       switch len(bytes) {
+       case 2:
+               return int(binary.BigEndian.Uint16(bytes)), nil
+       case 4:
+               return int(binary.BigEndian.Uint32(bytes)), nil
+       }
+
+       return 0, errors.New("unknown byte length")
+}

diff --git a/hotline/util_test.go b/hotline/util_test.go
new file mode 100644 (file)
index 0000000..30ae2b6
--- /dev/null
+++ b/hotline/util_test.go
@@ -0,0 +1,47 @@
+package hotline
+
+import (
+       "fmt"
+       "github.com/stretchr/testify/assert"
+       "testing"
+)
+
+func Test_byteToInt(t *testing.T) {
+       type args struct {
+               bytes []byte
+       }
+       tests := []struct {
+               name    string
+               args    args
+               want    int
+               wantErr assert.ErrorAssertionFunc
+       }{
+               {
+                       name:    "with 2 bytes of input",
+                       args:    args{bytes: []byte{0, 1}},
+                       want:    1,
+                       wantErr: assert.NoError,
+               },
+               {
+                       name:    "with 4 bytes of input",
+                       args:    args{bytes: []byte{0, 1, 0, 0}},
+                       want:    65536,
+                       wantErr: assert.NoError,
+               },
+               {
+                       name:    "with invalid number of bytes of input",
+                       args:    args{bytes: []byte{1, 0, 0, 0, 0, 0, 0, 0}},
+                       want:    0,
+                       wantErr: assert.Error,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       got, err := byteToInt(tt.args.bytes)
+                       if !tt.wantErr(t, err, fmt.Sprintf("byteToInt(%v)", tt.args.bytes)) {
+                               return
+                       }
+                       assert.Equalf(t, tt.want, got, "byteToInt(%v)", tt.args.bytes)
+               })
+       }
+}